Re: [PATCH 0/2] btrfs: convert btrfs_buffered_write() to folio interface

[Qu Wenruo <wqu@suse.com>]

在 2024/10/22 11:47, David Sterba 写道:
> On Thu, Oct 10, 2024 at 03:16:11PM +1030, Qu Wenruo wrote:
>> Inspired by generic_perform_write(), convert btrfs_buffered_write() to
>> go a page-by-page iteration, then convert it to use folio interface.
>>
>> This should provide the basis for use to go
>> address_operations->write_begin() and write_end() callbacks.
>>
>> There is a tiny timing change.
>> Before this patchset we wait for the page writeback after we get an
>> uptodate or no need to read the page.
>>
>> But now we follow the regular FGP_WRITEBEGIN, which implies FGP_STABLE
>> and will wait for writeback before reading the page.
>>
>> Qu Wenruo (2):
>>    btrfs: make buffered write to copy one page a time
>>    btrfs: convert btrfs_buffered_write() to use folio interface
> 
> So this is another step towards folios, ok. The first patch got a LKP
> bot report about performance degradataion due to the change from
> multiple pages to per page loop, but you mention that. The drop is 16%,
> that's not something that we should ignore. It was for one workload so
> this is mabye acceptable.

The performance drop in fact is larger than my expectation, and affects 
quite some benchmarks, thus I'm not in a rush to push it for upstream 
anytime soon.

> 
> Another thing is that how it's changed to the write begin/end, I don't
> understand this enough to give it a yes/no.

It's only one step towards that interface, and there are still quite 
some work left.

And the change to write_begin/end interface is not high on my to-do list.

> Also this is the buffered
> write so quite fundamental file operation. I've had the patches in
> misc-next (and in linux-next) for some time, so we have some testing. I
> hope we get more reviews too. We're now in rc4, so it's about the time
> to get it in or delay until the next cycle if there are doubts.
> 
> Regarding the patches, I have only style comments, you touch a lot of
> code that is not changed often so it would be good to fix the style of
> code or comments but correctness comes first so this can be fixed later,
> I'd really like to be sure this is safe.
> 
> My suggestion is to add the patches to for-next, we'll have enough time
> to catch problems and expose the new code to more testing environments.

OK, I can do the push, but if anyone hits any functional problems I'll 
be more than happier to revert/remove them immediately.

Thanks,
Qu