Re: [Patch v2 00/42] btrfs: add data write support for subpage

On 21/05/14 07:56AM, riteshh wrote:

On 21/05/14 09:41AM, Qu Wenruo wrote:

quoted

On 2021/5/14 上午6:54, David Sterba wrote:

quoted

On Thu, May 13, 2021 at 10:21:24AM +0800, Qu Wenruo wrote:

quoted

quoted

quoted

Do you think the patches 1-13 are safe to be merged independently? I've
paged through the whole patchset and some of the patches are obviously
preparatory stuff so they can go in without much risk.

Yes. I believe they are OK for merge.

I have run the full tests on x86 VM for the whole patchset, no new
regression.

Especially patch 03~05 would benefit 4K page size the most, thus merging
them first would definitely help.

Just let me to run the tests with patch 1~13 only, to see if there is
any special dependency missing.

Yep, patch 1~13 with the v5 read time repair patches are safe for x86.

So they should be fine for the next merge window.

quoted

quoted

I haven't looked at your git if there are updates from what was posted,
but I don't expect any significant changes, but what I saw looked ok to
me.

I haven't touched those patches since v2 submission, thus there
shouldn't be any modification to them.
(At most some cosmetic change for the commit message/comments)

quoted

If there are changes, please post 1-13 (ie. all the preparatory
patches), I'll put them to misc-next so you can focus on the rest.

I did another pass and found a few unimportant style fixes, it's now
pushed to branch ext/qu/subpage-prep-13. I'll run tests before merging
it to misc-next, the cleanups are great, some changes scare me a bit
though. Handling the ordered extents gets changed a bit, nothing
obviously wrong but based on past experience there are some subtle bugs
lurking.

Yes, that's also what I'm a little concerned of.

But with more understanding on ordered extent, it should be less a
concern, at least for x86.

Currently the biggest change is in the new
btrfs_mark_ordered_io_finished(), it will do extra skip for range
without Ordered (Private2) bit.

For x86 it shouldn't be a big problem as one page represents one sector,
and the only location we may get such call is for cases we don't need to
submit IO.

Those cases are fully covered by fstests, according to my countless
crashes/failures during initial tests.

Other than that, the btrfs_mark_ordered_io_finished() behavior should be
the same as old one, at least for x86.

Although more tests are always helpful.

If it helps, I tested "-g quick" on PPC64 with 64k config for 1-13 patches of
this patch series and didn't find any regression/crash with xfstests.
I am running "-g auto" now, will let you know the results once it completes.

I tested these patches (1-13) with "-g auto" config and I didn't see any
regression/crashes on PPC64 platform.

Thanks
ritesh

-ritesh

quoted

Thanks,
Qu

quoted

The plan is to add the branch to misc-next soon so we have enough time
to test it. I'll reply to the individual patches with comments that
stand out among the trivialities.