Re: [PATCH V2 00/12] xfs: Extend per-inode extent counters

[Chandan Babu R <chandanrlinux@gmail.com>]

On 29 Jul 2021 at 02:57, Darrick J. Wong wrote:
> On Mon, Jul 26, 2021 at 05:15:29PM +0530, Chandan Babu R wrote:
>> The commit xfs: fix inode fork extent count overflow
>> (3f8a4f1d876d3e3e49e50b0396eaffcc4ba71b08) mentions that 10 billion
>> data fork extents should be possible to create. However the
>> corresponding on-disk field has a signed 32-bit type. Hence this
>> patchset extends the per-inode data extent counter to 64 bits out of
>> which 48 bits are used to store the extent count.
>
> A few other random notes that don't fit anywhere:
>
> If you decide to enable upgrades by adding an XFS_DIFLAG2 to indicate
> that a particular file has large extent counts, I think you can copy the
> same mechanisms that the DIFLAG2_BIGTIME flag uses as a template.  I
> think you'd need an extra bit of logic in xfs_trans_log_inode to turn on
> the feature bit and move the n*extents fields around if either extent
> count exceeds the old limits.
>

Ok. I will start implementing the upgrade feature unless objections are raised
by other developers.

> Please export the NREXT64 feature flag via an XFS_FSOP_GEOM flag so that
> userspace can detect support for having a lot of extents.  This will
> make it easy for libfrog to figure out that it should set the "send
> large extent counts" bulkstat flag.  It'll also make it easier to
> perform feature detection in fstests.

Sure. I will implement this as well.

Thanks for the suggestions.

>
> --D
>
>> Also, XFS has an attr fork extent counter which is 16 bits wide. A
>> workload which,
>> 1. Creates 1 million 255-byte sized xattrs,
>> 2. Deletes 50% of these xattrs in an alternating manner,
>> 3. Tries to insert 400,000 new 255-byte sized xattrs
>>    causes the xattr extent counter to overflow.
>>
>> Dave tells me that there are instances where a single file has more
>> than 100 million hardlinks. With parent pointers being stored in
>> xattrs, we will overflow the signed 16-bits wide xattr extent counter
>> when large number of hardlinks are created. Hence this patchset
>> extends the on-disk field to 32-bits.
>>
>> The following changes are made to accomplish this,
>> 1. A new incompat superblock flag to prevent older kernels from mounting
>>    the filesystem. This flag has to be set during mkfs time.
>> 2. A new 64-bit inode field is created to hold the data extent
>>    counter.
>> 3. The existing 32-bit inode data extent counter will be used to hold
>>    the attr fork extent counter.
>>
>> The patchset has been tested by executing xfstests with the following
>> mkfs.xfs options,
>> 1. -m crc=0 -b size=1k
>> 2. -m crc=0 -b size=4k
>> 3. -m crc=0 -b size=512
>> 4. -m rmapbt=1,reflink=1 -b size=1k
>> 5. -m rmapbt=1,reflink=1 -b size=4k
>>
>> Each of the above test scenarios were executed on the following
>> combinations (For V4 FS test scenario, the last combination
>> i.e. "Patched (enable extcnt64bit)", was omitted).
>> |-------------------------------+-----------|
>> | Xfsprogs                      | Kernel    |
>> |-------------------------------+-----------|
>> | Unpatched                     | Patched   |
>> | Patched (disable extcnt64bit) | Unpatched |
>> | Patched (disable extcnt64bit) | Patched   |
>> | Patched (enable extcnt64bit)  | Patched   |
>> |-------------------------------+-----------|
>>
>> I have also written a test (yet to be converted into xfstests format)
>> to check if the correct extent counter fields are updated with/without
>> the new incompat flag. I have also fixed some of the existing fstests
>> to work with the new extent counter fields.
>>
>> Increasing data extent counter width also causes the maximum height of
>> BMBT to increase. This requires that the macro XFS_BTREE_MAXLEVELS be
>> updated with a larger value. However such a change causes the value of
>> mp->m_rmap_maxlevels to increase which in turn causes log reservation
>> sizes to increase and hence a modified XFS driver will fail to mount
>> filesystems created by older versions of mkfs.xfs.
>>
>> Hence this patchset is built on top of Darrick's btree-dynamic-depth
>> branch which removes the macro XFS_BTREE_MAXLEVELS and computes
>> mp->m_rmap_maxlevels based on the size of an AG.
>>
>> These patches can also be obtained from
>> https://github.com/chandanr/linux.git at branch
>> xfs-incompat-extend-extcnt-v2.
>>
>> I will be posting the changes associated with xfsprogs separately.
>>
>> Changelog:
>> V1 -> V2:
>> 1. Rebase patches on top of Darrick's btree-dynamic-depth branch.
>> 2. Add new bulkstat ioctl version to support 64-bit data fork extent
>>    counter field.
>> 3. Introduce new error tag to verify if the old bulkstat ioctls skip
>>    reporting inodes with large data fork extent counters.
>>
>> Chandan Babu R (12):
>>   xfs: Move extent count limits to xfs_format.h
>>   xfs: Rename MAXEXTNUM, MAXAEXTNUM to XFS_IFORK_EXTCNT_MAXS32,
>>     XFS_IFORK_EXTCNT_MAXS16
>>   xfs: Introduce xfs_iext_max() helper
>>   xfs: Use xfs_extnum_t instead of basic data types
>>   xfs: Introduce xfs_dfork_nextents() helper
>>   xfs: xfs_dfork_nextents: Return extent count via an out argument
>>   xfs: Rename inode's extent counter fields based on their width
>>   xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits
>>     respectively
>>   xfs: Rename XFS_IOC_BULKSTAT to XFS_IOC_BULKSTAT_V5
>>   xfs: Enable bulkstat ioctl to support 64-bit extent counters
>>   xfs: Extend per-inode extent counter widths
>>   xfs: Error tag to test if v5 bulkstat skips inodes with large extent
>>     count
>>
>>  fs/xfs/libxfs/xfs_bmap.c        | 21 +++----
>>  fs/xfs/libxfs/xfs_errortag.h    |  4 +-
>>  fs/xfs/libxfs/xfs_format.h      | 42 +++++++++++---
>>  fs/xfs/libxfs/xfs_fs.h          |  9 ++-
>>  fs/xfs/libxfs/xfs_inode_buf.c   | 82 ++++++++++++++++++++++++----
>>  fs/xfs/libxfs/xfs_inode_buf.h   |  2 +
>>  fs/xfs/libxfs/xfs_inode_fork.c  | 35 +++++++++---
>>  fs/xfs/libxfs/xfs_inode_fork.h  | 22 +++++++-
>>  fs/xfs/libxfs/xfs_log_format.h  |  7 ++-
>>  fs/xfs/libxfs/xfs_types.h       | 11 +---
>>  fs/xfs/scrub/attr_repair.c      |  2 +-
>>  fs/xfs/scrub/inode.c            | 97 ++++++++++++++++++++-------------
>>  fs/xfs/scrub/inode_repair.c     | 71 +++++++++++++++++-------
>>  fs/xfs/scrub/trace.h            | 16 +++---
>>  fs/xfs/xfs_error.c              |  3 +
>>  fs/xfs/xfs_inode.c              |  4 +-
>>  fs/xfs/xfs_inode_item.c         | 15 ++++-
>>  fs/xfs/xfs_inode_item_recover.c | 25 +++++++--
>>  fs/xfs/xfs_ioctl.c              | 33 +++++++++--
>>  fs/xfs/xfs_ioctl32.c            |  7 +++
>>  fs/xfs/xfs_itable.c             | 35 ++++++++++--
>>  fs/xfs/xfs_itable.h             |  1 +
>>  fs/xfs/xfs_trace.h              |  6 +-
>>  23 files changed, 402 insertions(+), 148 deletions(-)
>>
>> --
>> 2.30.2
>>

--
chandan