| Message ID | 20230110224906.1171483-3-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: filesystem population fixes | expand |
On Wed, Jan 11, 2023 at 09:49:05AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The population function creates an XFS btree format directory by > polling the extent count of the inode and creating new dirents until > the extent count goes over the limit that pushes it into btree > format. > > It then removes every second dirent to create empty space in the > directory data to ensure that operations like metadump with > obfuscation can check that they don't leak stale data from deleted > dirents. > > Whilst this does not result in directory data blocks being freed, it > does not take into account the fact that the dabtree index has half > the entries removed from it and that can result in btree nodes > merging and extents being freed. This causes the extent count to go > down, and the inode is converted back into extent form. The > population checks then fail because it should be in btree form. > > Fix this by counting the number of directory data extents rather than > the total number of extents in the data fork. We can do this simply > by using xfs_bmap and counting the number of extents returned as it > does not report extents beyond EOF (which is where the dabtree is > located). As the number of data blocks does not change with the > dirent removal algorithm used, this will ensure that the inode data > fork remains in btree format. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- > common/populate | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/populate b/common/populate > index 9b60fa5c1..7b5b16fb8 100644 > --- a/common/populate > +++ b/common/populate > @@ -80,8 +80,11 @@ __populate_create_nfiles() { > continue > fi > > - local nextents="$(_xfs_get_fsxattr nextents $name)" > - if [ "${nextents}" -gt "${max_nextents}" ]; then > + # Extent count checks use data blocks only to avoid the removal > + # step from removing dabtree index blocks and reducing the > + # number of extents below the required threshold. > + local nextents="$(xfs_bmap ${name} |grep -v hole | wc -l)" > + if [ "$((nextents - 1))" -gt "${max_nextents}" ]; then Pretty much the same patch I had in my tree... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > echo ${d} > break > fi > -- > 2.38.1 >
On 2023/1/11 06:49, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The population function creates an XFS btree format directory by > polling the extent count of the inode and creating new dirents until > the extent count goes over the limit that pushes it into btree > format. > > It then removes every second dirent to create empty space in the > directory data to ensure that operations like metadump with > obfuscation can check that they don't leak stale data from deleted > dirents. > > Whilst this does not result in directory data blocks being freed, it > does not take into account the fact that the dabtree index has half > the entries removed from it and that can result in btree nodes > merging and extents being freed. This causes the extent count to go > down, and the inode is converted back into extent form. The > population checks then fail because it should be in btree form. > > Fix this by counting the number of directory data extents rather than > the total number of extents in the data fork. We can do this simply > by using xfs_bmap and counting the number of extents returned as it > does not report extents beyond EOF (which is where the dabtree is > located). As the number of data blocks does not change with the > dirent removal algorithm used, this will ensure that the inode data > fork remains in btree format. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> Thanks, Gao Xiang > --- > common/populate | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/populate b/common/populate > index 9b60fa5c1..7b5b16fb8 100644 > --- a/common/populate > +++ b/common/populate > @@ -80,8 +80,11 @@ __populate_create_nfiles() { > continue > fi > > - local nextents="$(_xfs_get_fsxattr nextents $name)" > - if [ "${nextents}" -gt "${max_nextents}" ]; then > + # Extent count checks use data blocks only to avoid the removal > + # step from removing dabtree index blocks and reducing the > + # number of extents below the required threshold. > + local nextents="$(xfs_bmap ${name} |grep -v hole | wc -l)" > + if [ "$((nextents - 1))" -gt "${max_nextents}" ]; then > echo ${d} > break > fi
diff --git a/common/populate b/common/populate index 9b60fa5c1..7b5b16fb8 100644 --- a/common/populate +++ b/common/populate @@ -80,8 +80,11 @@ __populate_create_nfiles() { continue fi - local nextents="$(_xfs_get_fsxattr nextents $name)" - if [ "${nextents}" -gt "${max_nextents}" ]; then + # Extent count checks use data blocks only to avoid the removal + # step from removing dabtree index blocks and reducing the + # number of extents below the required threshold. + local nextents="$(xfs_bmap ${name} |grep -v hole | wc -l)" + if [ "$((nextents - 1))" -gt "${max_nextents}" ]; then echo ${d} break fi