Re: [PATCH 2/6] btrfs: Improve comments around nocow path

[Nikolay Borisov <nborisov@suse.com>]

On 6.08.19 г. 13:09 ч., Filipe Manana wrote:
> On Mon, Aug 5, 2019 at 3:48 PM Nikolay Borisov <nborisov@suse.com> wrote:

<snip>

>> @@ -1371,23 +1376,39 @@ static noinline int run_delalloc_nocow(struct inode *inode,
>>
>>                 btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
>>
>> +               /* Didn't find anything for our INO */
>>                 if (found_key.objectid > ino)
>>                         break;
>> +               /*
>> +                * Found a different inode or no extents for our file,
>> +                * goto next slot
> 
> No. This does not mean that there are no extents for the file. If
> there weren't any, we would break instead of iterating to the next
> slot.
> One example described at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1d512cb77bdbda80f0dd0620a3b260d697fd581d

I see, thanks for the pointer. How about the following :

/*
                 * Keep searching until we find an EXTENT ITEM or are
sure
                 * there are no more extents for this inode

                 */

While it doesn't mention the race condition this check, coupled with the
next one (where we break if type > EXTENT_DATA_KEY), it reflects reality
close enough?


> 
>> +                */
>>                 if (WARN_ON_ONCE(found_key.objectid < ino) ||
>>                     found_key.type < BTRFS_EXTENT_DATA_KEY) {
>>                         path->slots[0]++;
>>                         goto next_slot;
>>                 }
>> +
>> +               /* Found key is not EXTENT_DATA_KEY or starts after req range */
>>                 if (found_key.type > BTRFS_EXTENT_DATA_KEY ||
>>                     found_key.offset > end)
>>                         break;
>>
>> +               /*
>> +                * If the found extent starts after requested offset, then
>> +                * adjust extent_end to be right before this extent begins
>> +                */
>>                 if (found_key.offset > cur_offset) {
>>                         extent_end = found_key.offset;
>>                         extent_type = 0;
>>                         goto out_check;
>>                 }
>>
>> +
>> +               /*
>> +                * Found extent which begins before our range and has the
>> +                * potential to intersect it.
>> +                */
>>                 fi = btrfs_item_ptr(leaf, path->slots[0],
>>                                     struct btrfs_file_extent_item);
>>                 extent_type = btrfs_file_extent_type(leaf, fi);
<snip>