linux-ext4 - 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aL_US3g7BFpRccQE@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com>
Date: Tue, 9 Sep 2025 12:46:27 +0530
From: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
To: Zorro Lang <zlang@...hat.com>
Cc: John Garry <john.g.garry@...cle.com>, fstests@...r.kernel.org,
        Ritesh Harjani <ritesh.list@...il.com>, djwong@...nel.org,
        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 Sun, Sep 07, 2025 at 01:29:43PM +0800, Zorro Lang wrote:
> On Tue, Sep 02, 2025 at 03:50:10PM +0100, John Garry wrote:
> > On 22/08/2025 09:02, 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.
> > > 
> > > 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)
> > 
> > This requires the user to know the version which corresponds to the feature.
> > Is that how things are done for other such utilities and their versions vs
> > features?
> > 
> > I was going to suggest exporting something like
> > _require_fio_atomic_writes(), and _require_fio_atomic_writes() calls
> > _require_fio_version() to check the version.
> 
> (Sorry, I made a half reply in my last email)
> 
> This looks better than only using _require_fio_version. But the nature is still
> checking fio version. If we don't have a better idea to check if fio really
> support atomic writes, the _require_fio_version is still needed.
> Or we rename it to "__require_fio_version" (one more "_"), to mark it's
> not recommended using directly. But that looks a bit like a trick :-D
> 
> Thanks,
> Zorro

Hey Zorro, I agree with your points that version might not be the best
indicator esp for downstream software, but at this point I'm unsure
what's the workaround. 

One thing that comes to mind is to let fio do the atomic write and use
the tracepoints to confirm if RWF_ATOMIC was passed, but that adds a lot
of dependency on tracing framework being present (im unsure if something
like this is used somewhere in xfstests before). Further it's messy to
figure out that out of all the IO fio command will do, which one to
check for RWF_ATOMIC.

It can be done I suppose but is this sort of complexity something we
want to add is the question. Or do we just go ahead with the version
check.

Regards,
ojaswin

> 
> 
> > 
> > Thanks,
> > John
> > 
> > 
>