Re: [RFC 01/12] common/preamble: Fix fsx for ext4 with bigalloc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 20, 2025 at 11:51:07AM +0530, Ojaswin Mujoo wrote:
> On Thu, Jun 19, 2025 at 03:13:24AM +0800, Zorro Lang wrote:
> > On Wed, Jun 11, 2025 at 03:04:44PM +0530, Ojaswin Mujoo wrote:
> > > From: "Ritesh Harjani (IBM)" <ritesh.list@xxxxxxxxx>
> > > 
> > > 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 _setup_fs_options
> > > which can serve as a mechanism to apply FS-wide options to
> > > the tests.
> > > 
> > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> > > Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> > > ---
> > >  common/preamble | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/common/preamble b/common/preamble
> > > index ba029a34..2bccff74 100644
> > > --- a/common/preamble
> > > +++ b/common/preamble
> > > @@ -24,6 +24,20 @@ _register_cleanup()
> > >  	trap "${cleanup}exit \$status" EXIT HUP INT QUIT TERM $*
> > >  }
> > >  
> > > +# setup FS options only to be available for each test run
> > > +_setup_fs_options() {
> > 
> > If this's a function for fsx only, better to name it with "fsx", e.g.
> > _setup_default_fsx_avoid (or some other names).
> > 
> > > +	case "$FSTYP" in
> > > +	"ext4")
> > > +		if [[ "$MKFS_OPTIONS" =~ bigalloc ]]; then
> > > +			export FSX_AVOID="-I -C"
> > 
> > Hmm... I'm also wondering if this's an issue should be fixed in fstests. How about
> > let the testers who tests ext4 with MKFS_OPTIONS="-O bigalloc" write local.config
> > as below?
> > 
> > [ext4-bigalloc]
> > ...
> > MKFS_OPTIONS="-O bigalloc"
> > FSX_AVOID="-I -C"
> > 
> > Thanks,
> > Zorro
> 
> Hey Zorro, 
> 
> Basically the idea is that _setup_fs_options is a generic function that
> can be used to do any fs specific modifications to the global options.
> 
> This way we can set options to avoid known issues with different FSes,
> which can otherwise confuse the user if they are not aware of such
> issues. 
> 
> Does that sound okay?

I think we should give this choice to the user or the test case who wants
to test ext4 bigalloc feature. And MKFS_OPTIONS isn't the only way to
enable bigalloc, some test cases can do _scratch_mkfs "-O bigalloc".

**But, if *ext4 list* really hope to force set FSX_AVOID="-I -C" for
"$MKFS_OPTIONS" =~ bigalloc, then better to do this in _run_fsx* function at
first, I don't think it's worth having a global _setup_fs_options, and call
it at the beginning of each case running for now.

Thanks,
Zorro

> 
> Regards,
> ojaswin
> 
> > 
> > 
> > > +		fi
> > > +		;;
> > > +	# Add other filesystem types here as needed
> > > +	*)
> > > +		;;
> > > +	esac
> > > +}
> > > +
> > >  # Prepare to run a fstest by initializing the required global variables to
> > >  # their defaults, sourcing common functions, registering a cleanup function,
> > >  # and removing the $seqres.full file.
> > > @@ -55,4 +69,6 @@ _begin_fstest()
> > >  	# remove previous $seqres.full before test
> > >  	rm -f $seqres.full $seqres.hints
> > >  
> > > +	# setup filesystem options for a given test execution
> > > +	_setup_fs_options
> > >  }
> > > -- 
> > > 2.49.0
> > > 
> > > 
> > 
> 





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux