linux-kernel - Re: [PATCH v5 02/12] common/rc: Add _require_fio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aLHcgyWtwqMTX-Mz@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com>
Date: Fri, 29 Aug 2025 22:29:47 +0530
From: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Zorro Lang <zlang@...hat.com>, fstests@...r.kernel.org,
        Ritesh Harjani <ritesh.list@...il.com>, john.g.garry@...cle.com,
        tytso@....edu, linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH v5 02/12] common/rc: Add _require_fio_version helper

On Thu, Aug 28, 2025 at 08:09:05AM -0700, Darrick J. Wong wrote:
> On Wed, Aug 27, 2025 at 08:46:34PM +0530, Ojaswin Mujoo wrote:
> > On Tue, Aug 26, 2025 at 12:08:01AM +0800, Zorro Lang wrote:
> > > On Fri, Aug 22, 2025 at 01:32:01PM +0530, Ojaswin Mujoo wrote:
> > > > The main motivation of adding this function on top of _require_fio is
> > > > that there has been a case in fio where atomic= option was added but
> > > > later it was changed to noop since kernel didn't yet have support for
> > > > atomic writes. It was then again utilized to do atomic writes in a later
> > > > version, once kernel got the support. Due to this there is a point in
> > > > fio where _require_fio w/ atomic=1 will succeed even though it would
> > > > not be doing atomic writes.
> > > > 
> > > > Hence, add an explicit helper to ensure tests to require specific
> > > > versions of fio to work past such issues.
> > > 
> > > Actually I'm wondering if fstests really needs to care about this. This's
> > > just a temporary issue of fio, not kernel or any fs usespace program. Do
> > > we need to add a seperated helper only for a temporary fio issue? If fio
> > > doesn't break fstests running, let it run. Just the testers install proper
> > > fio (maybe latest) they need. What do you and others think?
> 
> Are there obvious failures if you try to run these new atomic write
> tests on a system with the weird versions of fio that have the no-op
> atomic= functionality?  I'm concerned that some QA person is going to do
> that unwittingly and report that everything is ok when in reality they
> didn't actually test anything.

I think John has a bit more background but afaict, RWF_ATOMIC support
was added (fio commit: d01612f3ae25) but then removed (commit:
a25ba6c64fe1) since the feature didn't make it to kernel in time.
However the option seemed to be kept in place. Later, commit 40f1fc11d
added the support back in a later version of fio. 

So yes, I think there are some version where fio will accept atomic=1
but not act upon it and the tests may start failing with no apparent
reason.

Regards,
ojaswin
> 
> --D
> 
> > > Thanks,
> > > Zorro
> > 
> > Hey Zorro,
> > 
> > Sure I'm okay with not keeping the helper and letting the user make sure
> > the fio version is correct.
> > 
> > @John, does that sound okay?
> > 
> > Regards,
> > ojaswin
> > > 
> > > > 
> > > > Signed-off-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
> > > > ---
> > > >  common/rc | 32 ++++++++++++++++++++++++++++++++
> > > >  1 file changed, 32 insertions(+)
> > > > 
> > > > diff --git a/common/rc b/common/rc
> > > > index 35a1c835..f45b9a38 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -5997,6 +5997,38 @@ _max() {
> > > >  	echo $ret
> > > >  }
> > > >  
> > > > +# Check the required fio version. Examples:
> > > > +#   _require_fio_version 3.38 (matches 3.38 only)
> > > > +#   _require_fio_version 3.38+ (matches 3.38 and above)
> > > > +#   _require_fio_version 3.38- (matches 3.38 and below)
> > > > +_require_fio_version() {
> > > > +	local req_ver="$1"
> > > > +	local fio_ver
> > > > +
> > > > +	_require_fio
> > > > +	_require_math
> > > > +
> > > > +	fio_ver=$(fio -v | cut -d"-" -f2)
> > > > +
> > > > +	case "$req_ver" in
> > > > +	*+)
> > > > +		req_ver=${req_ver%+}
> > > > +		test $(_math "$fio_ver >= $req_ver") -eq 1 || \
> > > > +			_notrun "need fio >= $req_ver (found $fio_ver)"
> > > > +		;;
> > > > +	*-)
> > > > +		req_ver=${req_ver%-}
> > > > +		test $(_math "$fio_ver <= $req_ver") -eq 1 || \
> > > > +			_notrun "need fio <= $req_ver (found $fio_ver)"
> > > > +		;;
> > > > +	*)
> > > > +		req_ver=${req_ver%-}
> > > > +		test $(_math "$fio_ver == $req_ver") -eq 1 || \
> > > > +			_notrun "need fio = $req_ver (found $fio_ver)"
> > > > +		;;
> > > > +	esac
> > > > +}
> > > > +
> > > >  ################################################################################
> > > >  # make sure this script returns success
> > > >  /bin/true
> > > > -- 
> > > > 2.49.0
> > > > 
> > > 
> >