* [PATCH V3 0/2] cleanup and bugfix for xfs tests related to btree format @ 2022-12-06 10:05 Ziyang Zhang 2022-12-06 10:05 ` [PATCH V3 1/2] common/xfs: Add a helper to export inode core size Ziyang Zhang 2022-12-06 10:05 ` [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang 0 siblings, 2 replies; 7+ messages in thread From: Ziyang Zhang @ 2022-12-06 10:05 UTC (permalink / raw) To: fstests, linux-xfs; +Cc: djwong, hsiangkao, allison.henderson, Ziyang Zhang The first patch add a helper in common/xfs to export the inode core size which is needed by some xfs test cases. The second patch ensure that S_IFDIR.FMT_BTREE is in btree format while populating dir. V3: - refactor xfs tests cases using inode core size V2: - take Darrick's advice to cleanup code Ziyang Zhang (2): common/xfs: Add a helper to export inode core size common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format common/populate | 28 +++++++++++++++++++++++++++- common/xfs | 16 ++++++++++++++++ tests/xfs/335 | 3 ++- tests/xfs/336 | 3 ++- tests/xfs/337 | 3 ++- tests/xfs/341 | 3 ++- tests/xfs/342 | 3 ++- 7 files changed, 53 insertions(+), 6 deletions(-) -- 2.18.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3 1/2] common/xfs: Add a helper to export inode core size 2022-12-06 10:05 [PATCH V3 0/2] cleanup and bugfix for xfs tests related to btree format Ziyang Zhang @ 2022-12-06 10:05 ` Ziyang Zhang 2022-12-06 17:18 ` Darrick J. Wong 2022-12-06 23:37 ` Allison Henderson 2022-12-06 10:05 ` [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang 1 sibling, 2 replies; 7+ messages in thread From: Ziyang Zhang @ 2022-12-06 10:05 UTC (permalink / raw) To: fstests, linux-xfs; +Cc: djwong, hsiangkao, allison.henderson, Ziyang Zhang Some xfs test cases need the number of bytes reserved for only the inode record, excluding the immediate fork areas. Now the value is hard-coded and it is not a good chioce. Add a helper in common/xfs to export the inode core size. Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> --- common/xfs | 7 +++++++ tests/xfs/335 | 3 ++- tests/xfs/336 | 3 ++- tests/xfs/337 | 3 ++- tests/xfs/341 | 3 ++- tests/xfs/342 | 3 ++- 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/common/xfs b/common/xfs index 8ac1964e..5180b9d3 100644 --- a/common/xfs +++ b/common/xfs @@ -1486,3 +1486,10 @@ _require_xfsrestore_xflag() $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \ _notrun 'xfsrestore does not support -x flag.' } + +# Number of bytes reserved for only the inode record, excluding the +# immediate fork areas. +_xfs_inode_core_bytes() +{ + echo 176 +} diff --git a/tests/xfs/335 b/tests/xfs/335 index ccc508e7..624a8fd1 100755 --- a/tests/xfs/335 +++ b/tests/xfs/335 @@ -31,7 +31,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" echo "Create a three-level rtrmapbt" # inode core size is at least 176 bytes; btree header is 56 bytes; # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. -i_ptrs=$(( (isize - 176) / 56 )) +i_core_size="$(_xfs_inode_core_bytes)" +i_ptrs=$(( (isize - i_core_size) / 56 )) bt_ptrs=$(( (blksz - 56) / 56 )) bt_recs=$(( (blksz - 56) / 32 )) diff --git a/tests/xfs/336 b/tests/xfs/336 index b1de8e5f..e601632d 100755 --- a/tests/xfs/336 +++ b/tests/xfs/336 @@ -42,7 +42,8 @@ rm -rf $metadump_file echo "Create a three-level rtrmapbt" # inode core size is at least 176 bytes; btree header is 56 bytes; # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. -i_ptrs=$(( (isize - 176) / 56 )) +i_core_size="$(_xfs_inode_core_bytes)" +i_ptrs=$(( (isize - i_core_size) / 56 )) bt_ptrs=$(( (blksz - 56) / 56 )) bt_recs=$(( (blksz - 56) / 32 )) diff --git a/tests/xfs/337 b/tests/xfs/337 index a2515e36..5d5ec8dc 100755 --- a/tests/xfs/337 +++ b/tests/xfs/337 @@ -33,7 +33,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" # inode core size is at least 176 bytes; btree header is 56 bytes; # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. -i_ptrs=$(( (isize - 176) / 56 )) +i_core_size="$(_xfs_inode_core_bytes)" +i_ptrs=$(( (isize - i_core_size) / 56 )) bt_ptrs=$(( (blksz - 56) / 56 )) bt_recs=$(( (blksz - 56) / 32 )) diff --git a/tests/xfs/341 b/tests/xfs/341 index f026aa37..45afd407 100755 --- a/tests/xfs/341 +++ b/tests/xfs/341 @@ -33,7 +33,8 @@ rtextsz_blks=$((rtextsz / blksz)) # inode core size is at least 176 bytes; btree header is 56 bytes; # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. -i_ptrs=$(( (isize - 176) / 56 )) +i_core_size="$(_xfs_inode_core_bytes)" +i_ptrs=$(( (isize - i_core_size) / 56 )) bt_recs=$(( (blksz - 56) / 32 )) blocks=$((i_ptrs * bt_recs + 1)) diff --git a/tests/xfs/342 b/tests/xfs/342 index 1ae414eb..d4f54168 100755 --- a/tests/xfs/342 +++ b/tests/xfs/342 @@ -30,7 +30,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" # inode core size is at least 176 bytes; btree header is 56 bytes; # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. -i_ptrs=$(( (isize - 176) / 56 )) +i_core_size="$(_xfs_inode_core_bytes)" +i_ptrs=$(( (isize - i_core_size) / 56 )) bt_recs=$(( (blksz - 56) / 32 )) blocks=$((i_ptrs * bt_recs + 1)) -- 2.18.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3 1/2] common/xfs: Add a helper to export inode core size 2022-12-06 10:05 ` [PATCH V3 1/2] common/xfs: Add a helper to export inode core size Ziyang Zhang @ 2022-12-06 17:18 ` Darrick J. Wong 2022-12-06 23:37 ` Allison Henderson 1 sibling, 0 replies; 7+ messages in thread From: Darrick J. Wong @ 2022-12-06 17:18 UTC (permalink / raw) To: Ziyang Zhang; +Cc: fstests, linux-xfs, hsiangkao, allison.henderson On Tue, Dec 06, 2022 at 06:05:16PM +0800, Ziyang Zhang wrote: > Some xfs test cases need the number of bytes reserved for only the inode > record, excluding the immediate fork areas. Now the value is hard-coded > and it is not a good chioce. Add a helper in common/xfs to export the > inode core size. > > Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> Looks good, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > common/xfs | 7 +++++++ > tests/xfs/335 | 3 ++- > tests/xfs/336 | 3 ++- > tests/xfs/337 | 3 ++- > tests/xfs/341 | 3 ++- > tests/xfs/342 | 3 ++- > 6 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/common/xfs b/common/xfs > index 8ac1964e..5180b9d3 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -1486,3 +1486,10 @@ _require_xfsrestore_xflag() > $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \ > _notrun 'xfsrestore does not support -x flag.' > } > + > +# Number of bytes reserved for only the inode record, excluding the > +# immediate fork areas. > +_xfs_inode_core_bytes() > +{ > + echo 176 > +} > diff --git a/tests/xfs/335 b/tests/xfs/335 > index ccc508e7..624a8fd1 100755 > --- a/tests/xfs/335 > +++ b/tests/xfs/335 > @@ -31,7 +31,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > echo "Create a three-level rtrmapbt" > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/336 b/tests/xfs/336 > index b1de8e5f..e601632d 100755 > --- a/tests/xfs/336 > +++ b/tests/xfs/336 > @@ -42,7 +42,8 @@ rm -rf $metadump_file > echo "Create a three-level rtrmapbt" > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/337 b/tests/xfs/337 > index a2515e36..5d5ec8dc 100755 > --- a/tests/xfs/337 > +++ b/tests/xfs/337 > @@ -33,7 +33,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/341 b/tests/xfs/341 > index f026aa37..45afd407 100755 > --- a/tests/xfs/341 > +++ b/tests/xfs/341 > @@ -33,7 +33,8 @@ rtextsz_blks=$((rtextsz / blksz)) > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > blocks=$((i_ptrs * bt_recs + 1)) > diff --git a/tests/xfs/342 b/tests/xfs/342 > index 1ae414eb..d4f54168 100755 > --- a/tests/xfs/342 > +++ b/tests/xfs/342 > @@ -30,7 +30,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > blocks=$((i_ptrs * bt_recs + 1)) > -- > 2.18.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 1/2] common/xfs: Add a helper to export inode core size 2022-12-06 10:05 ` [PATCH V3 1/2] common/xfs: Add a helper to export inode core size Ziyang Zhang 2022-12-06 17:18 ` Darrick J. Wong @ 2022-12-06 23:37 ` Allison Henderson 1 sibling, 0 replies; 7+ messages in thread From: Allison Henderson @ 2022-12-06 23:37 UTC (permalink / raw) To: fstests@vger.kernel.org, linux-xfs@vger.kernel.org, ZiyangZhang@linux.alibaba.com Cc: djwong@kernel.org, hsiangkao@linux.alibaba.com On Tue, 2022-12-06 at 18:05 +0800, Ziyang Zhang wrote: > Some xfs test cases need the number of bytes reserved for only the > inode > record, excluding the immediate fork areas. Now the value is hard- > coded > and it is not a good chioce. Add a helper in common/xfs to export the > inode core size. > > Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> Looks clean to me Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > --- > common/xfs | 7 +++++++ > tests/xfs/335 | 3 ++- > tests/xfs/336 | 3 ++- > tests/xfs/337 | 3 ++- > tests/xfs/341 | 3 ++- > tests/xfs/342 | 3 ++- > 6 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/common/xfs b/common/xfs > index 8ac1964e..5180b9d3 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -1486,3 +1486,10 @@ _require_xfsrestore_xflag() > $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \ > _notrun 'xfsrestore does not support -x > flag.' > } > + > +# Number of bytes reserved for only the inode record, excluding the > +# immediate fork areas. > +_xfs_inode_core_bytes() > +{ > + echo 176 > +} > diff --git a/tests/xfs/335 b/tests/xfs/335 > index ccc508e7..624a8fd1 100755 > --- a/tests/xfs/335 > +++ b/tests/xfs/335 > @@ -31,7 +31,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > echo "Create a three-level rtrmapbt" > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/336 b/tests/xfs/336 > index b1de8e5f..e601632d 100755 > --- a/tests/xfs/336 > +++ b/tests/xfs/336 > @@ -42,7 +42,8 @@ rm -rf $metadump_file > echo "Create a three-level rtrmapbt" > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/337 b/tests/xfs/337 > index a2515e36..5d5ec8dc 100755 > --- a/tests/xfs/337 > +++ b/tests/xfs/337 > @@ -33,7 +33,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_ptrs=$(( (blksz - 56) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > diff --git a/tests/xfs/341 b/tests/xfs/341 > index f026aa37..45afd407 100755 > --- a/tests/xfs/341 > +++ b/tests/xfs/341 > @@ -33,7 +33,8 @@ rtextsz_blks=$((rtextsz / blksz)) > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > blocks=$((i_ptrs * bt_recs + 1)) > diff --git a/tests/xfs/342 b/tests/xfs/342 > index 1ae414eb..d4f54168 100755 > --- a/tests/xfs/342 > +++ b/tests/xfs/342 > @@ -30,7 +30,8 @@ blksz="$(_get_block_size $SCRATCH_MNT)" > > # inode core size is at least 176 bytes; btree header is 56 bytes; > # rtrmap record is 32 bytes; and rtrmap key/pointer are 56 bytes. > -i_ptrs=$(( (isize - 176) / 56 )) > +i_core_size="$(_xfs_inode_core_bytes)" > +i_ptrs=$(( (isize - i_core_size) / 56 )) > bt_recs=$(( (blksz - 56) / 32 )) > > blocks=$((i_ptrs * bt_recs + 1)) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format 2022-12-06 10:05 [PATCH V3 0/2] cleanup and bugfix for xfs tests related to btree format Ziyang Zhang 2022-12-06 10:05 ` [PATCH V3 1/2] common/xfs: Add a helper to export inode core size Ziyang Zhang @ 2022-12-06 10:05 ` Ziyang Zhang 2022-12-06 17:29 ` Darrick J. Wong 1 sibling, 1 reply; 7+ messages in thread From: Ziyang Zhang @ 2022-12-06 10:05 UTC (permalink / raw) To: fstests, linux-xfs; +Cc: djwong, hsiangkao, allison.henderson, Ziyang Zhang Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that S_IFDIR.FMT_BTREE could become btree format for its DATA fork. Actually we just observed it can fail after apply our inode extent-to-btree workaround. The root cause is that the kernel may be too good at allocating consecutive blocks so that the data fork is still in extents format. Therefore instead of using a fixed number, let's make sure the number of extents is large enough than (inode size - inode core size) / sizeof(xfs_bmbt_rec_t). Reviewed-by: Allison Henderson <allison.henderson@oracle.com> Suggested-by: "Darrick J. Wong" <djwong@kernel.org> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> --- common/populate | 28 +++++++++++++++++++++++++++- common/xfs | 9 +++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/common/populate b/common/populate index 6e004997..1ca76459 100644 --- a/common/populate +++ b/common/populate @@ -71,6 +71,31 @@ __populate_create_dir() { done } +# Create a large directory and ensure that it's a btree format +__populate_xfs_create_btree_dir() { + local name="$1" + local isize="$2" + local icore_size="$(_xfs_inode_core_bytes)" + # We need enough extents to guarantee that the data fork is in + # btree format. Cycling the mount to use xfs_db is too slow, so + # watch for when the extent count exceeds the space after the + # inode core. + local max_nextents="$(((isize - icore_size) / 16))" + + mkdir -p "${name}" + d=0 + while true; do + creat=mkdir + test "$((d % 20))" -eq 0 && creat=touch + $creat "${name}/$(printf "%.08d" "$d")" + if [ "$((d % 40))" -eq 0 ]; then + nextents="$(_xfs_get_fsxattr nextents $name)" + [ $nextents -gt $max_nextents ] && break + fi + d=$((d+1)) + done +} + # Add a bunch of attrs to a file __populate_create_attr() { name="$1" @@ -176,6 +201,7 @@ _scratch_xfs_populate() { blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")" dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")" + isize="$(_xfs_inode_size "$SCRATCH_MNT")" crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)" if [ $crc -eq 1 ]; then leaf_hdr_size=64 @@ -226,7 +252,7 @@ _scratch_xfs_populate() { # - BTREE echo "+ btree dir" - __populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true + __populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" # Symlinks # - FMT_LOCAL diff --git a/common/xfs b/common/xfs index 5180b9d3..744f0040 100644 --- a/common/xfs +++ b/common/xfs @@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag() _notrun 'xfsrestore does not support -x flag.' } +# Number of bytes reserved for a full inode record, which includes the +# immediate fork areas. +_xfs_inode_size() +{ + local mntpoint="$1" + + $XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g' +} + # Number of bytes reserved for only the inode record, excluding the # immediate fork areas. _xfs_inode_core_bytes() -- 2.18.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format 2022-12-06 10:05 ` [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang @ 2022-12-06 17:29 ` Darrick J. Wong 2022-12-07 18:02 ` Zorro Lang 0 siblings, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2022-12-06 17:29 UTC (permalink / raw) To: Ziyang Zhang; +Cc: fstests, linux-xfs, hsiangkao, allison.henderson On Tue, Dec 06, 2022 at 06:05:17PM +0800, Ziyang Zhang wrote: > Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that > S_IFDIR.FMT_BTREE could become btree format for its DATA fork. > > Actually we just observed it can fail after apply our inode > extent-to-btree workaround. The root cause is that the kernel may be > too good at allocating consecutive blocks so that the data fork is > still in extents format. > > Therefore instead of using a fixed number, let's make sure the number > of extents is large enough than (inode size - inode core size) / > sizeof(xfs_bmbt_rec_t). > > Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > Suggested-by: "Darrick J. Wong" <djwong@kernel.org> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> > Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> > --- > common/populate | 28 +++++++++++++++++++++++++++- > common/xfs | 9 +++++++++ > 2 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/common/populate b/common/populate > index 6e004997..1ca76459 100644 > --- a/common/populate > +++ b/common/populate > @@ -71,6 +71,31 @@ __populate_create_dir() { > done > } > > +# Create a large directory and ensure that it's a btree format > +__populate_xfs_create_btree_dir() { > + local name="$1" > + local isize="$2" > + local icore_size="$(_xfs_inode_core_bytes)" > + # We need enough extents to guarantee that the data fork is in > + # btree format. Cycling the mount to use xfs_db is too slow, so > + # watch for when the extent count exceeds the space after the > + # inode core. > + local max_nextents="$(((isize - icore_size) / 16))" > + > + mkdir -p "${name}" > + d=0 > + while true; do > + creat=mkdir > + test "$((d % 20))" -eq 0 && creat=touch > + $creat "${name}/$(printf "%.08d" "$d")" > + if [ "$((d % 40))" -eq 0 ]; then > + nextents="$(_xfs_get_fsxattr nextents $name)" > + [ $nextents -gt $max_nextents ] && break > + fi > + d=$((d+1)) > + done > +} > + > # Add a bunch of attrs to a file > __populate_create_attr() { > name="$1" > @@ -176,6 +201,7 @@ _scratch_xfs_populate() { > > blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")" > dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")" > + isize="$(_xfs_inode_size "$SCRATCH_MNT")" > crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)" > if [ $crc -eq 1 ]; then > leaf_hdr_size=64 > @@ -226,7 +252,7 @@ _scratch_xfs_populate() { > > # - BTREE > echo "+ btree dir" > - __populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true > + __populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" The new helper function omits the "missing" parameter, which means that it no longer creates the directory entry blocks with a lot of free space in them, unlike current TOT. --D > > # Symlinks > # - FMT_LOCAL > diff --git a/common/xfs b/common/xfs > index 5180b9d3..744f0040 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag() > _notrun 'xfsrestore does not support -x flag.' > } > > +# Number of bytes reserved for a full inode record, which includes the > +# immediate fork areas. > +_xfs_inode_size() > +{ > + local mntpoint="$1" > + > + $XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g' > +} > + > # Number of bytes reserved for only the inode record, excluding the > # immediate fork areas. > _xfs_inode_core_bytes() > -- > 2.18.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format 2022-12-06 17:29 ` Darrick J. Wong @ 2022-12-07 18:02 ` Zorro Lang 0 siblings, 0 replies; 7+ messages in thread From: Zorro Lang @ 2022-12-07 18:02 UTC (permalink / raw) To: Ziyang Zhang Cc: Darrick J. Wong, fstests, linux-xfs, hsiangkao, allison.henderson On Tue, Dec 06, 2022 at 09:29:54AM -0800, Darrick J. Wong wrote: > On Tue, Dec 06, 2022 at 06:05:17PM +0800, Ziyang Zhang wrote: > > Sometimes "$((128 * dblksz / 40))" dirents cannot make sure that > > S_IFDIR.FMT_BTREE could become btree format for its DATA fork. > > > > Actually we just observed it can fail after apply our inode > > extent-to-btree workaround. The root cause is that the kernel may be > > too good at allocating consecutive blocks so that the data fork is > > still in extents format. > > > > Therefore instead of using a fixed number, let's make sure the number > > of extents is large enough than (inode size - inode core size) / > > sizeof(xfs_bmbt_rec_t). > > > > Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > > Suggested-by: "Darrick J. Wong" <djwong@kernel.org> > > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> > > Signed-off-by: Ziyang Zhang <ZiyangZhang@linux.alibaba.com> > > --- > > common/populate | 28 +++++++++++++++++++++++++++- > > common/xfs | 9 +++++++++ > > 2 files changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/common/populate b/common/populate > > index 6e004997..1ca76459 100644 > > --- a/common/populate > > +++ b/common/populate > > @@ -71,6 +71,31 @@ __populate_create_dir() { > > done > > } > > > > +# Create a large directory and ensure that it's a btree format > > +__populate_xfs_create_btree_dir() { > > + local name="$1" > > + local isize="$2" > > + local icore_size="$(_xfs_inode_core_bytes)" > > + # We need enough extents to guarantee that the data fork is in > > + # btree format. Cycling the mount to use xfs_db is too slow, so > > + # watch for when the extent count exceeds the space after the > > + # inode core. > > + local max_nextents="$(((isize - icore_size) / 16))" > > + > > + mkdir -p "${name}" > > + d=0 > > + while true; do > > + creat=mkdir > > + test "$((d % 20))" -eq 0 && creat=touch > > + $creat "${name}/$(printf "%.08d" "$d")" > > + if [ "$((d % 40))" -eq 0 ]; then > > + nextents="$(_xfs_get_fsxattr nextents $name)" > > + [ $nextents -gt $max_nextents ] && break > > + fi > > + d=$((d+1)) > > + done > > +} > > + > > # Add a bunch of attrs to a file > > __populate_create_attr() { > > name="$1" > > @@ -176,6 +201,7 @@ _scratch_xfs_populate() { > > > > blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")" > > dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")" > > + isize="$(_xfs_inode_size "$SCRATCH_MNT")" > > crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)" > > if [ $crc -eq 1 ]; then > > leaf_hdr_size=64 > > @@ -226,7 +252,7 @@ _scratch_xfs_populate() { > > > > # - BTREE > > echo "+ btree dir" > > - __populate_create_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$((128 * dblksz / 40))" true > > + __populate_xfs_create_btree_dir "${SCRATCH_MNT}/S_IFDIR.FMT_BTREE" "$isize" > > The new helper function omits the "missing" parameter, which means that > it no longer creates the directory entry blocks with a lot of free space > in them, unlike current TOT. Hi Ziyang, Is there any reason we must drop this "missing" step. If no special reason, I'd like to keep the original operation (the missing parameter is "true") to free some space sparsely. I think that only removes some entries, won't remove the dir-blocks (generally), so it won't affect the btree format you create. Thanks, Zorro > > --D > > > > > # Symlinks > > # - FMT_LOCAL > > diff --git a/common/xfs b/common/xfs > > index 5180b9d3..744f0040 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -1487,6 +1487,15 @@ _require_xfsrestore_xflag() > > _notrun 'xfsrestore does not support -x flag.' > > } > > > > +# Number of bytes reserved for a full inode record, which includes the > > +# immediate fork areas. > > +_xfs_inode_size() > > +{ > > + local mntpoint="$1" > > + > > + $XFS_INFO_PROG "$mntpoint" | grep 'meta-data=.*isize' | sed -e 's/^.*isize=\([0-9]*\).*$/\1/g' > > +} > > + > > # Number of bytes reserved for only the inode record, excluding the > > # immediate fork areas. > > _xfs_inode_core_bytes() > > -- > > 2.18.4 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-12-07 18:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-06 10:05 [PATCH V3 0/2] cleanup and bugfix for xfs tests related to btree format Ziyang Zhang 2022-12-06 10:05 ` [PATCH V3 1/2] common/xfs: Add a helper to export inode core size Ziyang Zhang 2022-12-06 17:18 ` Darrick J. Wong 2022-12-06 23:37 ` Allison Henderson 2022-12-06 10:05 ` [PATCH V3 2/2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang 2022-12-06 17:29 ` Darrick J. Wong 2022-12-07 18:02 ` Zorro Lang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox