Re: [PATCH v3 02/13] common/rc: Fix fsx for ext4 with bigalloc

Ojaswin Mujoo
2025-07-22T09:53:22Z

On Thu, Jul 17, 2025 at 09:11:54AM -0700, Darrick J. Wong wrote:
> On Sat, Jul 12, 2025 at 07:42:44PM +0530, Ojaswin Mujoo wrote:
> > Insert range and collapse range only works with bigalloc in case
> > the range is cluster size aligned, which fsx doesnt take care. To
> > work past this, disable insert range and collapse range on ext4, if
> > bigalloc is enabled.
> >
> > This is achieved by defining a new function _set_default_fsx_avoid
> > called via run_fsx helper. This can be used to selectively disable
> > fsx options based on the configuration.
> >
> > Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> > common/rc | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/common/rc b/common/rc
> > index 9a9d3cc8..218cf253 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -5113,10 +5113,37 @@ _require_hugepage_fsx()
> > _notrun "fsx binary does not support MADV_COLLAPSE"
> > }
> >
> > +_set_default_fsx_avoid() {
> > + local file=$1
> > +
> > + case "$FSTYP" in
> > + "ext4")
> > + local dev=$(findmnt -n -o SOURCE --target $file)
> > +
> > + # open code instead of _require_dumpe2fs cause we don't
> > + # want to _notrun if dumpe2fs is not available
> > + if [ -z "$DUMPE2FS_PROG" ]; then
> > + echo "_set_default_fsx_avoid: dumpe2fs not found, skipping bigalloc check." >> $seqres.full
> > + return
> > + fi
>
> I hate to be the guy who says one thing and then another, but ...
>
> If we extended _get_file_block_size to report the ext4 bigalloc cluster
> size, would that be sufficient to keep testing collapse/insert range?
>
> I guess the tricky part here is that bigalloc allows sub-cluster
> mappings and we might not want to do all file IO testing in such big
> units.

Hmm, so maybe a better way is to just add a parameter like alloc_unit in
fsx where we can pass the cluster_size to which INSERT/COLLAPSE range be
aligned to. For now we can pass it explicitly in the tests if needed.

I do plan on working on your suggestion of exposing alloc unit via
statx(). Once we have that in the kernel, fsx can use that as well.

If this approach sounds okay I can try to maybe send the whole "fixing
of insert/collpase range in fsx" as a patchset separate from atomic
writes.

>
> > +
> > + $DUMPE2FS_PROG -h $dev 2>&1 | grep -q bigalloc && {
> > + export FSX_AVOID+=" -I -C"
>
> No need to export FSX_AVOID to subprocesses.
>
> --D

Got it, will fix. Thanks for review!

Regards,
ojaswin
>
> > + }
> > + ;;
> > + # Add other filesystem types here as needed
> > + *)
> > + ;;
> > + esac
> > +}
> > +
> > _run_fsx()
> > {
> > echo "fsx $*"
> > local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> > +
> > + _set_default_fsx_avoid $testfile
> > +
> > set -- $FSX_PROG $args $FSX_AVOID $TEST_DIR/junk
> > echo "$@" >>$seqres.full
> > rm -f $TEST_DIR/junk
> > --
> > 2.49.0
> >
> >