* [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format
@ 2022-12-02 11:27 Ziyang Zhang
2022-12-03 0:26 ` Allison Henderson
0 siblings, 1 reply; 5+ messages in thread
From: Ziyang Zhang @ 2022-12-02 11:27 UTC (permalink / raw)
To: fstests, linux-xfs; +Cc: djwong, Ziyang Zhang, Gao Xiang
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).
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>
---
V2: take Darrick's advice to cleanup code
common/populate | 28 +++++++++++++++++++++++++++-
common/xfs | 17 +++++++++++++++++
2 files changed, 44 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 8ac1964e..0359e422 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1486,3 +1486,20 @@ _require_xfsrestore_xflag()
$XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
_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()
+{
+ echo 176
+}
--
2.18.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format
2022-12-02 11:27 [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang
@ 2022-12-03 0:26 ` Allison Henderson
2022-12-05 21:51 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Allison Henderson @ 2022-12-03 0:26 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 Fri, 2022-12-02 at 19:27 +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).
>
> 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>
New version looks much cleaner.
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> V2: take Darrick's advice to cleanup code
> common/populate | 28 +++++++++++++++++++++++++++-
> common/xfs | 17 +++++++++++++++++
> 2 files changed, 44 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 8ac1964e..0359e422 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1486,3 +1486,20 @@ _require_xfsrestore_xflag()
> $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
> _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()
> +{
> + echo 176
> +}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format
2022-12-03 0:26 ` Allison Henderson
@ 2022-12-05 21:51 ` Darrick J. Wong
2022-12-06 3:13 ` Ziyang Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2022-12-05 21:51 UTC (permalink / raw)
To: Allison Henderson
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org,
ZiyangZhang@linux.alibaba.com, hsiangkao@linux.alibaba.com
On Sat, Dec 03, 2022 at 12:26:57AM +0000, Allison Henderson wrote:
> On Fri, 2022-12-02 at 19:27 +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).
> >
> > 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>
>
> New version looks much cleaner.
> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
>
> > ---
> > V2: take Darrick's advice to cleanup code
> > common/populate | 28 +++++++++++++++++++++++++++-
> > common/xfs | 17 +++++++++++++++++
> > 2 files changed, 44 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 8ac1964e..0359e422 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -1486,3 +1486,20 @@ _require_xfsrestore_xflag()
> > $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
> > _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()
> > +{
> > + echo 176
> > +}
Please refactor all the other users:
$ git grep -w isize.*176
tests/xfs/335:34:i_ptrs=$(( (isize - 176) / 56 ))
tests/xfs/336:45:i_ptrs=$(( (isize - 176) / 56 ))
tests/xfs/337:36:i_ptrs=$(( (isize - 176) / 56 ))
tests/xfs/341:36:i_ptrs=$(( (isize - 176) / 48 ))
tests/xfs/342:33:i_ptrs=$(( (isize - 176) / 56 ))
(I'll test out this patch and try to do the same for ATTR.FMT_BTREE in
the meantime)
--D
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format
2022-12-05 21:51 ` Darrick J. Wong
@ 2022-12-06 3:13 ` Ziyang Zhang
2022-12-06 3:32 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Ziyang Zhang @ 2022-12-06 3:13 UTC (permalink / raw)
To: Darrick J. Wong
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org,
hsiangkao@linux.alibaba.com, Allison Henderson
On 2022/12/6 05:51, Darrick J. Wong wrote:
> On Sat, Dec 03, 2022 at 12:26:57AM +0000, Allison Henderson wrote:
>> On Fri, 2022-12-02 at 19:27 +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).
>>>
>>> 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>
>>
>> New version looks much cleaner.
>> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
>>
>>> ---
>>> V2: take Darrick's advice to cleanup code
>>> common/populate | 28 +++++++++++++++++++++++++++-
>>> common/xfs | 17 +++++++++++++++++
>>> 2 files changed, 44 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 8ac1964e..0359e422 100644
>>> --- a/common/xfs
>>> +++ b/common/xfs
>>> @@ -1486,3 +1486,20 @@ _require_xfsrestore_xflag()
>>> $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
>>> _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()
>>> +{
>>> + echo 176
>>> +}
>
> Please refactor all the other users:
>
> $ git grep -w isize.*176
> tests/xfs/335:34:i_ptrs=$(( (isize - 176) / 56 ))
> tests/xfs/336:45:i_ptrs=$(( (isize - 176) / 56 ))
> tests/xfs/337:36:i_ptrs=$(( (isize - 176) / 56 ))
> tests/xfs/341:36:i_ptrs=$(( (isize - 176) / 48 ))
> tests/xfs/342:33:i_ptrs=$(( (isize - 176) / 56 ))
>
> (I'll test out this patch and try to do the same for ATTR.FMT_BTREE in
> the meantime)
Hi, Darrick
Should I create a new patch to refactor these test cases
or refactor them just in this patch?
Looks like these test cases just need _xfs_inode_core_bytes()
and commit message of this patch is not written for them.
Regards,
Zhang
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format
2022-12-06 3:13 ` Ziyang Zhang
@ 2022-12-06 3:32 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2022-12-06 3:32 UTC (permalink / raw)
To: Ziyang Zhang
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org,
hsiangkao@linux.alibaba.com, Allison Henderson
On Tue, Dec 06, 2022 at 11:13:10AM +0800, Ziyang Zhang wrote:
> On 2022/12/6 05:51, Darrick J. Wong wrote:
> > On Sat, Dec 03, 2022 at 12:26:57AM +0000, Allison Henderson wrote:
> >> On Fri, 2022-12-02 at 19:27 +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).
> >>>
> >>> 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>
> >>
> >> New version looks much cleaner.
> >> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> >>
> >>> ---
> >>> V2: take Darrick's advice to cleanup code
> >>> common/populate | 28 +++++++++++++++++++++++++++-
> >>> common/xfs | 17 +++++++++++++++++
> >>> 2 files changed, 44 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 8ac1964e..0359e422 100644
> >>> --- a/common/xfs
> >>> +++ b/common/xfs
> >>> @@ -1486,3 +1486,20 @@ _require_xfsrestore_xflag()
> >>> $XFSRESTORE_PROG -h 2>&1 | grep -q -e '-x' || \
> >>> _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()
> >>> +{
> >>> + echo 176
> >>> +}
> >
> > Please refactor all the other users:
> >
> > $ git grep -w isize.*176
> > tests/xfs/335:34:i_ptrs=$(( (isize - 176) / 56 ))
> > tests/xfs/336:45:i_ptrs=$(( (isize - 176) / 56 ))
> > tests/xfs/337:36:i_ptrs=$(( (isize - 176) / 56 ))
> > tests/xfs/341:36:i_ptrs=$(( (isize - 176) / 48 ))
> > tests/xfs/342:33:i_ptrs=$(( (isize - 176) / 56 ))
> >
> > (I'll test out this patch and try to do the same for ATTR.FMT_BTREE in
> > the meantime)
>
> Hi, Darrick
>
> Should I create a new patch to refactor these test cases
> or refactor them just in this patch?
>
> Looks like these test cases just need _xfs_inode_core_bytes()
> and commit message of this patch is not written for them.
Depends on what Zorro says, but yes, usually we'd want to refactor the
open-coded logic into a helper in a single patch, and after that comes
the bugfix.
--D
> Regards,
> Zhang
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-06 3:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-02 11:27 [RESEND PATCH V2] common/populate: Ensure that S_IFDIR.FMT_BTREE is in btree format Ziyang Zhang
2022-12-03 0:26 ` Allison Henderson
2022-12-05 21:51 ` Darrick J. Wong
2022-12-06 3:13 ` Ziyang Zhang
2022-12-06 3:32 ` Darrick J. Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox