Re: [PATCH v10 06/14] btrfs: optionally extend i_size in cow_file_range_inline()

Qu Wenruo
2021-08-21T01:11:44Z

On 2021/8/21 上午2:11, Omar Sandoval wrote:
> On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2021/8/20 下午4:51, Nikolay Borisov wrote:
>>>
>>>
>>> On 18.08.21 г. 0:06, Omar Sandoval wrote:
>>>> From: Omar Sandoval <osandov@fb.com>
>>>>
>>>> Currently, an inline extent is always created after i_size is extended
>>>> from btrfs_dirty_pages(). However, for encoded writes, we only want to
>>>> update i_size after we successfully created the inline extent.
>>
>> To me, the idea of write first then update isize is just going to cause
>> tons of inline extent related prblems.
>>
>> The current example is falloc, which only update the isize after the
>> falloc finishes.
>>
>> This behavior has already bothered me quite a lot, as it can easily
>> create mixed inline and regular extents.
>
> Do you have an example of how this would happen? I have the inode and
> extent bits locked during an encoded write, and I see that fallocate
> does the same.

xfs_io -f -c "pwrite 0 1K" -c "sync" -c "falloc 0 4k" -c "pwrite 4k 4k"

The [0, 1K) will be written as inline without doubt.

Then we go to falloc, it will try to zero the range [1K, 4K), but it
doesn't increase the isize.
Thus the page [0, 4k) will still be written back as inline, since isize
is still 1K.

Later [4K, 8K) will be written back as regular, causing mixed extents.

>
>> Can't we remember the old isize (with proper locking), enlarge isize
>> (with holes filled), do the write.
>>
>> If something wrong happened, we truncate the isize back to its old isize.
>>
[...]
>>>
>>> Urgh, just some days ago Qu was talking about how awkward it is to have
>>> mixed extents in a file. And now, AFAIU, you are making them more likely
>>> since now they can be created not just at the beginning of the file but
>>> also after i_size write. While this won't be a problem in and of itself
>>> it goes just the opposite way of us trying to shrink the possible cases
>>> when we can have mixed extents.
>>
>> Tree-checker should reject such inline extent at non-zero offset.
>
> This change does not allow creating inline extents at a non-zero offset.
>
>>> Qu what is your take on that?
>>
>> My question is, why encoded write needs to bother the inline extents at all?
>>
>> My intuition of such encoded write is, it should not create inline
>> extents at all.
>>
>> Or is there any special use-case involved for encoded write?
>
> We create compressed inline extents with normal writes. We should be
> able to send and receive them without converting them into regular
> extents.
>
But my first impression for any encoded write is that, they should work
like DIO, thus everything should be sectorsize aligned.

Then why could they create inline extent? As inline extent can only be
possible when the isize is smaller than sectorsize.

Thanks,
Qu