On Tue, Jul 22, 2025 at 03:23:02PM +0530, Ojaswin Mujoo wrote:
> 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@xxxxxxxxx>
> > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> > > ---
> > > 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.
Yeah, that sounds like a good longer-term solution to me. :)
--D
> >
> > > +
> > > + $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
> > >
> > >
>
