On 21/03/29 10:01AM, Qu Wenruo wrote: > > > On 2021/3/29 上午4:02, Ritesh Harjani wrote: > > On 21/03/25 09:16PM, Qu Wenruo wrote: > > > > > > > > > On 2021/3/25 下午8:20, Neal Gompa wrote: > > > > On Thu, Mar 25, 2021 at 3:17 AM Qu Wenruo <w...@suse.com> wrote: > > > > > > > > > > This patchset can be fetched from the following github repo, along > > > > > with > > > > > the full subpage RW support: > > > > > https://github.com/adam900710/linux/tree/subpage > > > > > > > > > > This patchset is for metadata read write support. > > > > > > > > > > [FULL RW TEST] > > > > > Since the data write path is not included in this patchset, we can't > > > > > really test the patchset itself, but anyone can grab the patch from > > > > > github repo and do fstests/generic tests. > > > > > > > > > > But at least the full RW patchset can pass -g generic/quick -x defrag > > > > > for now. > > > > > > > > > > There are some known issues: > > > > > > > > > > - Defrag behavior change > > > > > Since current defrag is doing per-page defrag, to support subpage > > > > > defrag, we need some change in the loop. > > > > > E.g. if a page has both hole and regular extents in it, then > > > > > defrag > > > > > will rewrite the full 64K page. > > > > > > > > > > Thus for now, defrag related failure is expected. > > > > > But this should only cause behavior difference, no crash nor hang > > > > > is > > > > > expected. > > > > > > > > > > - No compression support yet > > > > > There are at least 2 known bugs if forcing compression for subpage > > > > > * Some hard coded PAGE_SIZE screwing up space rsv > > > > > * Subpage ASSERT() triggered > > > > > This is because some compression code is unlocking locked_page > > > > > by > > > > > calling extent_clear_unlock_delalloc() with locked_page == NULL. > > > > > So for now compression is also disabled. > > > > > > > > > > - Inode nbytes mismatch > > > > > Still debugging. > > > > > The fastest way to trigger is fsx using the following parameters: > > > > > > > > > > fsx -l 262144 -o 65536 -S 30073 -N 256 -R -W $mnt/file > > > > > > /tmp/fsx > > > > > > > > > > Which would cause inode nbytes differs from expected value and > > > > > triggers btrfs check error. > > > > > > > > > > [DIFFERENCE AGAINST REGULAR SECTORSIZE] > > > > > The metadata part in fact has more new code than data part, as it has > > > > > some different behaviors compared to the regular sector size handling: > > > > > > > > > > - No more page locking > > > > > Now metadata read/write relies on extent io tree locking, other > > > > > than > > > > > page locking. > > > > > This is to allow behaviors like read lock one eb while also try to > > > > > read lock another eb in the same page. > > > > > We can't rely on page lock as now we have multiple extent buffers > > > > > in > > > > > the same page. > > > > > > > > > > - Page status update > > > > > Now we use subpage wrappers to handle page status update. > > > > > > > > > > - How to submit dirty extent buffers > > > > > Instead of just grabbing extent buffer from page::private, we > > > > > need to > > > > > iterate all dirty extent buffers in the page and submit them. > > > > > > > > > > [CHANGELOG] > > > > > v2: > > > > > - Rebased to latest misc-next > > > > > No conflicts at all. > > > > > > > > > > - Add new sysfs interface to grab supported RO/RW sectorsize > > > > > This will allow mkfs.btrfs to detect unmountable fs better. > > > > > > > > > > - Use newer naming schema for each patch > > > > > No more "extent_io:" or "inode:" schema anymore. > > > > > > > > > > - Move two pure cleanups to the series > > > > > Patch 2~3, originally in RW part. > > > > > > > > > > - Fix one uninitialized variable > > > > > Patch 6. > > > > > > > > > > v3: > > > > > - Rename the sysfs to supported_sectorsizes > > > > > > > > > > - Rebased to latest misc-next branch > > > > > This removes 2 cleanup patches. > > > > > > > > > > - Add new overview comment for subpage metadata > > > > > > > > > > Qu Wenruo (13): > > > > > btrfs: add sysfs interface for supported sectorsize > > > > > btrfs: use min() to replace open-code in btrfs_invalidatepage() > > > > > btrfs: remove unnecessary variable shadowing in > > > > > btrfs_invalidatepage() > > > > > btrfs: refactor how we iterate ordered extent in > > > > > btrfs_invalidatepage() > > > > > btrfs: introduce helpers for subpage dirty status > > > > > btrfs: introduce helpers for subpage writeback status > > > > > btrfs: allow btree_set_page_dirty() to do more sanity check on > > > > > subpage > > > > > metadata > > > > > btrfs: support subpage metadata csum calculation at write time > > > > > btrfs: make alloc_extent_buffer() check subpage dirty bitmap > > > > > btrfs: make the page uptodate assert to be subpage compatible > > > > > btrfs: make set/clear_extent_buffer_dirty() to be subpage > > > > > compatible > > > > > btrfs: make set_btree_ioerr() accept extent buffer and to be > > > > > subpage > > > > > compatible > > > > > btrfs: add subpage overview comments > > > > > > > > > > fs/btrfs/disk-io.c | 143 > > > > > ++++++++++++++++++++++++++++++++++--------- > > > > > fs/btrfs/extent_io.c | 127 ++++++++++++++++++++++++++++---------- > > > > > fs/btrfs/inode.c | 128 ++++++++++++++++++++++---------------- > > > > > fs/btrfs/subpage.c | 127 ++++++++++++++++++++++++++++++++++++++ > > > > > fs/btrfs/subpage.h | 17 +++++ > > > > > fs/btrfs/sysfs.c | 15 +++++ > > > > > 6 files changed, 441 insertions(+), 116 deletions(-) > > > > > > > > > > -- > > > > > 2.30.1 > > > > > > > > > > > > > Why wouldn't we just integrate full read-write support with the > > > > caveats as described now? It seems to be relatively reasonable to do > > > > that, and this patch set is essentially unusable without the rest of > > > > it that does enable full read-write support. > > > > > > The metadata part is much more stable than data path (almost not touched > > > for several months), and the metadata part already has some difference > > > in its behavior, which needs review. > > > > > > You point makes some sense, but I still don't believe pushing a super > > > large patchset does any help for the review. > > > > > > If you want to test, you can grab the branch from the github repo. > > > If you want to review, the mails are all here for review. > > > > > > In fact, we used to have subpage support sent as a big patchset from IBM > > > guys, but the result is only some preparation patches get merged, and > > > nothing more. > > > > > > Using this multi-series method, we're already doing better work and > > > received more testing (to ensure regular sectorsize is not affected at > > > least). > > > > Hi Qu Wenruo, > > > > Sorry about chiming in late on this. I don't have any strong objection on > > either > > approach. Although sometime back when I tested your RW support git tree on > > Power, the unmount patch itself was crashing. I didn't debug it that time > > (this was a month back or so), so I also didn't bother testing xfstests on > > Power. > > > > But we do have an interest in making sure this patch series work on bs < ps > > on Power platform. I can try helping with testing, reviewing (to best of my > > knowledge) and fixing anything is possible :) > > That's great! > > One of my biggest problem here is, I don't have good enough testing > environment. > > Although SUSE has internal clouds for ARM64/PPC64, but due to the > f**king Great Firewall, it's super slow to access, no to mention doing > proper debugging. > > Currently I'm using two ARM SBCs, RK3399 and A311D based, to do the test. > But their computing power is far from ideal, only generic/quick can > finish in hours. > > Thus real world Power could definitely help. > > > > Let me try and pull your tree and test it on Power. Please let me know if > > there > > is anything needs to be taken care apart from your github tree and > > btrfs-progs > > branch with bs < ps support. > > If you're going to test the branch, here are some small notes: > > - Need to use latest btrfs-progs > As it fixes a false alert on crossing 64K page boundary. > > - Need to slightly modify btrfs-progs to avoid false alerts > For subpage case, mkfs.btrfs will output a warning, but that warning > is outputted into stderr, which will screw up generic test groups. > It's recommended to apply the following diff: > > diff --git a/common/fsfeatures.c b/common/fsfeatures.c > index 569208a9..21976554 100644 > --- a/common/fsfeatures.c > +++ b/common/fsfeatures.c > @@ -341,8 +341,8 @@ int btrfs_check_sectorsize(u32 sectorsize) > return -EINVAL; > } > if (page_size != sectorsize) > - warning( > -"the filesystem may not be mountable, sectorsize %u doesn't match page > size %u", > + printf( > +"the filesystem may not be mountable, sectorsize %u doesn't match page > size %u\n", > sectorsize, page_size); > return 0; > } > > - Xfstest/btrfs group will crash at btrfs/143 > Still investigating, but you can ignore btrfs group for now. > > - Very rare hang > There is a very low change to hang, with "bad ordered accounting" > dmesg. > If you can hit, please let me know. > I had something idea to fix it, but not yet in the branch. > > - btrfs inode nbytes mismatch > Investigating, as it will make btrfs-check to report error. > > The last two bugs are the final show blocker, I'll give you extra > updates when those are fixed.
Thanks Qu Wenruo, for above info. I cloned below git tree as mentioned in your git log to test for RW on Power. However, I still see that RW mount for bs < ps is disabled for in open_ctree() https://github.com/adam900710/linux/tree/subpage I see below code present in this tree. /* For 4K sector size support, it's only read-only */ if (PAGE_SIZE == SZ_64K && sectorsize == SZ_4K) { if (!sb_rdonly(sb) || btrfs_super_log_root(disk_super)) { btrfs_err(fs_info, "subpage sectorsize %u only supported read-only for page size %lu", sectorsize, PAGE_SIZE); err = -EINVAL; goto fail_alloc; } } Could you pls point me to the tree I can use for bs < ps testing on Power? Sorry if I missed something. Thanks -ritesh
