On Fri, Dec 09, 2022 at 08:37:41AM -0800, Darrick J. Wong wrote:
> On Thu, Dec 08, 2022 at 03:28:43PM +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: Zorro Lang <zlang@xxxxxxxxxx>
> > Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > Suggested-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Ziyang Zhang <ZiyangZhang@xxxxxxxxxxxxxxxxx>
> > ---
> > common/populate | 34 +++++++++++++++++++++++++++++++++-
> > common/xfs | 9 +++++++++
> > 2 files changed, 42 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/populate b/common/populate
> > index 6e004997..0d334a13 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -71,6 +71,37 @@ __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 missing="$3"
> > + local icore_size="$(_xfs_inode_core_bytes)"
>
> Doesn't this helper require a path argument now?
What kind of "path" argument? I think he copy it from __populate_create_dir(),
and keep using the "name" as the root path to create files/dir.
>
> --D
>
> > + # 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))"
> > + local nr=0
> > +
> > + mkdir -p "${name}"
> > + while true; do
> > + local creat=mkdir
> > + test "$((nr % 20))" -eq 0 && creat=touch
> > + $creat "${name}/$(printf "%.08d" "$nr")"
> > + if [ "$((nr % 40))" -eq 0 ]; then
> > + local nextents="$(_xfs_get_fsxattr nextents $name)"
> > + [ $nextents -gt $max_nextents ] && break
> > + fi
> > + nr=$((nr+1))
> > + done
> > +
> > + test -z "${missing}" && return
> > + seq 1 2 "${nr}" | while read d; do
> > + rm -rf "${name}/$(printf "%.08d" "$d")"
> > + done
> > +}
> > +
> > # Add a bunch of attrs to a file
> > __populate_create_attr() {
> > name="$1"
> > @@ -176,6 +207,7 @@ _scratch_xfs_populate() {
> >
> > blksz="$(stat -f -c '%s' "${SCRATCH_MNT}")"
> > dblksz="$(_xfs_get_dir_blocksize "$SCRATCH_MNT")"
> > + isize="$(_xfs_get_inode_size "$SCRATCH_MNT")"
> > crc="$(_xfs_has_feature "$SCRATCH_MNT" crc -v)"
> > if [ $crc -eq 1 ]; then
> > leaf_hdr_size=64
> > @@ -226,7 +258,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" true
> >
> > # Symlinks
> > # - FMT_LOCAL
> > diff --git a/common/xfs b/common/xfs
> > index 674384a9..7aaa63c7 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_get_inode_size()
> > +{
> > + local mntpoint="$1"
> > +
> > + $XFS_INFO_PROG "$mntpoint" | sed -n '/meta-data=.*isize/s/^.*isize=\([0-9]*\).*$/\1/p'
> > +}
> > +
> > # Number of bytes reserved for only the inode record, excluding the
> > # immediate fork areas.
> > _xfs_get_inode_core_bytes()
> > --
> > 2.18.4
> >
>
