On Wed, Mar 4, 2020 at 6:55 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> On 3/4/20 5:34 AM, fdmanana@xxxxxxxxxx wrote:
> > From: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > Commit 0c713cbab6200b ("Btrfs: fix race between ranged fsync and writeback
> > of adjacent ranges") fixed a bug where we could end up with file extent
> > items in a log tree that represent file ranges that overlap due to a race
> > between the hole detection of a ranged full fsync and writeback for a
> > different file range.
> >
> > The problem was solved by forcing any ranged full fsync to become a
> > non-ranged full fsync - setting the range start to 0 and the end offset to
> > LLONG_MAX. This was a simple solution because the code that detected and
> > marked holes was very complex, it used to be done at copy_items() and
> > implied several searches on the fs/subvolume tree. The drawback of that
> > solution was that we started to flush delalloc for the entire file and
> > wait for all the ordered extents to complete for ranged full fsyncs
> > (including ordered extents covering ranges completely outside the given
> > range). Fortunatelly ranged full fsyncs are not the most common case.
> >
> > However a later fix for detecting and marking holes was made by commit
> > 0e56315ca147b3 ("Btrfs: fix missing hole after hole punching and fsync
> > when using NO_HOLES") and it simplified a lot the detection of holes,
> > and now copy_items() no longer does it and we do it in a much more simple
> > way at btrfs_log_holes(). This makes it now possible to simply make the
> > code that detects holes to operate only on the initial range and no longer
> > need to operate on the whole file, while also avoiding the need to flush
> > delalloc for the entire file and wait for ordered extents that cover
> > ranges that don't overlap the given range.
> >
> > So this change just does that, making any ranged full fsync to actually
> > operate only on the given range and not the whole file.
> >
> > Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
So this actually isn't safe.
It can bring back the race that leads to file extent items with
overlapping ranges. Not because of the hole detection part but because
of the part where we copy extent items from the fs/subvolume tree into
the log tree using btrfs_search_forward(), as we copy all extent
items, including the ones outside the fsync range - so we could race
in the same way as we did during hole detection with ordered extent
completion for ordered extents outside the range.
I'll have to rework this a bit.
>
> Thanks,
>
> Josef
