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