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: <20250902053052.ajw6nr2yqbrk4qmf@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
Date: Tue, 2 Sep 2025 13:30:52 +0800
From: Zorro Lang <zlang@...hat.com>
To: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
Cc: "Darrick J. Wong" <djwong@...nel.org>, 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 Mon, Sep 01, 2025 at 05:10:01PM +0530, Ojaswin Mujoo wrote:
> On Sun, Aug 31, 2025 at 01:09:07AM +0800, Zorro Lang wrote:
> > On Fri, Aug 29, 2025 at 10:29:47PM +0530, Ojaswin Mujoo wrote:
> > > 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.
> > 
> > The concern from Darrick might be a problem. May I ask which fio commit
> > brought in this issue, and which fio commit fixed it? If this issue be
> > brought in and fixed within a fio release, it might be better. But if it
> > crosses fio release, that might be bad, then we might be better to have
> > this helper.
> 
> Hi Zorro, yes it does seem to cross version boundaries. The
> functionality was removed in fio v3.33 and added back in v3.38.  I

Thanks, if so I think let's have this helper for that issue :) But I think
we still prioritize _require_fio. If it helpless, then call _require_fio_version.

Reviewed-by: Zorro Lang <zlang@...hat.com>

Thanks,
Zorro

> confirmed this by running generic/1226 with both (for v3.33 run i
> commented out a few fio options that were added later but kept
> atomic=1):
> 
> Command: sudo perf record -e iomap:iomap_dio_rw_begin ./check generic/1226
> 
> perf script sample with fio v3.33:
> 
> fio    6626 [000]   777.668017: iomap:iomap_dio_rw_begin: <.sniip.> flags DIRECT|WRITE|AIO_RW dio_flags  aio 1
> 
> perf script sample with fio v3.39:
> 
> fio    9830 [000]   895.042747: iomap:iomap_dio_rw_begin: <.snip> flags ATOMIC|DIRECT|WRITE|AIO_RW dio_flags  aio 1
> 
> So as we can see, even though the test passes with atomic=1, fio is not
> sending the RWF_ATOMIC flag in the older version.
> 
> In which case I believe it should be okay to keep the helper, right?
> 
> Thanks,
> Ojaswin
> 
> > 
> > Thanks,
> > Zorro
> > 
> > > 
> > > Regards,
> > > ojaswin
> > > > 
> > > > --D
> > > > 
> > > > > > Thanks,
> > > > > > Zorro
>