public inbox for fstests@vger.kernel.org
 help / color / mirror / Atom feed
From: John Garry <john.g.garry@oracle.com>
To: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Cc: Zorro Lang <zlang@redhat.com>,
	fstests@vger.kernel.org, Ritesh Harjani <ritesh.list@gmail.com>,
	djwong@kernel.org, tytso@mit.edu
Subject: Re: [PATCH v2 05/13] generic/1226: Add atomic write test using fio crc check verifier
Date: Tue, 8 Jul 2025 12:11:06 +0100	[thread overview]
Message-ID: <7b79cd75-317c-4b64-b98d-2f35a014deb8@oracle.com> (raw)
In-Reply-To: <aGy_xT5powRmYzjY@li-dc0c254c-257c-11b2-a85c-98b6c1322444.ibm.com>

On 08/07/2025 07:50, Ojaswin Mujoo wrote:
>>> Further, handling awu_max vs awu_max_opt and their different
>>> interpretations for XFS vs EXT4 etc are adding uneccessary complexity.
>> You could assume awu max opt == awu max for ext4, as we no there are no
>> software-based atomic writes. That could be simpler than my previous
>> suggestion, which involves a kernel change.
> I want to avoid adding something like
> 
> iosize = FSTYP == "ext4" ? awu_max : awu_max_opt
> 
> in the test iteslf, because it is a bit difficult to remember changing
> this in case things change in the kernel later, eg if ext4 gets a
> fallback.

You could do the statx call on the mounted bdev, and see if atomics are 
supported there. If not, skip the test.

But even for xfs when HW support is available, we can still use software 
fallback when HW support is not possible, i.e. misaligned. So, in that 
regard, we really should only test this on ext4.

> 
> Im not sure if returning non-zero for ext4 is also correct as per the
> documented behavior from 5d894321c49e613.
> 
>     When zero, it means that there is no such performance boundary.
> 
> Anyways, I think there might be a simpler solution...
>>> The simplest way seems to be to just limit the $blocksize to
>>> something like $(_min 16KB awu_max) and hope that 16KB is small enough
>>> to not be split during reads. We anyways have other tests like
>>> generic/1230 that can be used to test larger atomic write sizes
>>>
>>> Thoughts?
>> As I said at the start, we never had guarantees of serialization of reads
>> and atomic writes.
>>
>> However I still think that this is a useful test. It's just it is
>> theoretically possible to give false positives.
>>
>> You could get the test to read max_sectors_kb, and check whether it is
>> greater than the bs. Again, more complexity.
> I think this might work. So i can set the iosize as 16kb which is
> already low enought. Incase the max_sectors is < than 16kb then we bail
> out. (but then maybe there is a very small chance of read split still)

Sure, that seems fine.

> 
> OR another approach is to do the verify at the very end when all threads
> are done writing so the reads don't race. The tradeoff is that this
> will reduce the effictiveness of the test though to some extent.

Yeah, right. We would want this test to prove that atomic writes are not 
getting split by the block stack, so racing reads can help prove that.

> 
>> As an alternative, maybe it's better to maintain this test out-of-tree. I'm
>> not sure.
> That'll be a hassle for everyone :p Does any of the above 2 approaches
> seem acceptable for upstreaming this test?

Sure, I think ether are fine, but I am still a bit hesitant to even have 
this test for xfs (and not just ext4).

Thanks,
John



  reply	other threads:[~2025-07-08 11:11 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-26 11:58 [PATCH v2 00/13] Add more tests for multi fs block atomic writes Ojaswin Mujoo
2025-06-26 11:58 ` [PATCH v2 01/13] common/rc: Add _min() and _max() helpers Ojaswin Mujoo
2025-06-26 11:58 ` [PATCH v2 02/13] common/rc: Fix fsx for ext4 with bigalloc Ojaswin Mujoo
2025-06-26 13:32   ` Theodore Ts'o
2025-06-30 15:28     ` Darrick J. Wong
2025-07-01  6:26       ` Ojaswin Mujoo
2025-07-02 15:13         ` Darrick J. Wong
2025-06-26 11:58 ` [PATCH v2 03/13] common/rc: Add a helper to run fsx on a given file Ojaswin Mujoo
2025-06-26 11:58 ` [PATCH v2 04/13] ltp/fsx.c: Add atomic writes support to fsx Ojaswin Mujoo
2025-06-26 11:58 ` [PATCH v2 05/13] generic/1226: Add atomic write test using fio crc check verifier Ojaswin Mujoo
2025-06-27 14:09   ` John Garry
2025-07-01 16:18     ` Ojaswin Mujoo
2025-07-02  7:46       ` John Garry
2025-07-03  6:42         ` Ojaswin Mujoo
2025-07-03 16:26           ` John Garry
2025-07-04 14:35             ` Ojaswin Mujoo
2025-07-04 15:23               ` Ojaswin Mujoo
2025-07-07  8:18                 ` John Garry
2025-07-08  6:50                   ` Ojaswin Mujoo
2025-07-08 11:11                     ` John Garry [this message]
2025-07-08 12:01                       ` Ojaswin Mujoo
2025-07-08 12:34                         ` John Garry
2025-07-11 10:39                           ` Ojaswin Mujoo
2025-07-11 10:51                             ` John Garry
2025-07-11 18:16                               ` Ojaswin Mujoo
2025-07-07  8:02               ` John Garry
2025-06-26 11:58 ` [PATCH v2 06/13] generic/1227: Add atomic write test using fio verify on file mixed mappings Ojaswin Mujoo
2025-06-27 14:48   ` John Garry
2025-06-26 11:58 ` [PATCH v2 07/13] generic/1228: Add atomic write multi-fsblock O_[D]SYNC tests Ojaswin Mujoo
2025-06-26 11:58 ` [PATCH v2 08/13] generic/1229: Stress fsx with atomic writes enabled Ojaswin Mujoo
2025-06-26 11:59 ` [PATCH v2 09/13] generic/1230: Add sudden shutdown tests for multi block atomic writes Ojaswin Mujoo
2025-06-27 16:11   ` John Garry
2025-07-01  6:34     ` Ojaswin Mujoo
2025-06-26 11:59 ` [PATCH v2 10/13] ext4/061: Atomic writes stress test for bigalloc using fio crc verifier Ojaswin Mujoo
2025-06-26 11:59 ` [PATCH v2 11/13] ext4/062: Atomic writes test for bigalloc using fio crc verifier on multiple files Ojaswin Mujoo
2025-06-26 11:59 ` [PATCH v2 12/13] ext4/063: Atomic write test for extent split across leaf nodes Ojaswin Mujoo
2025-06-26 11:59 ` [PATCH v2 13/13] ext4/064: Add atomic write tests for journal credit calculation Ojaswin Mujoo
2025-06-27 13:56 ` [PATCH v2 00/13] Add more tests for multi fs block atomic writes John Garry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7b79cd75-317c-4b64-b98d-2f35a014deb8@oracle.com \
    --to=john.g.garry@oracle.com \
    --cc=djwong@kernel.org \
    --cc=fstests@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=tytso@mit.edu \
    --cc=zlang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox