Re: [PATCH v3 05/13] generic/1226: Add atomic write test using fio crc check verifier

Ojaswin Mujoo
2025-07-28T13:35:44Z

On Mon, Jul 28, 2025 at 10:09:47AM +0100, John Garry wrote:
> On 28/07/2025 07:43, Ojaswin Mujoo wrote:
> > > > What do you mean by not safe?
> > > Multiple threads issuing atomic writes may trample over one another.
> > >
> > > It is due to the steps used to issue an atomic write in xfs by software
> > > method. Here we do 3x steps:
> > > a. allocate blocks for out-of-place write
> > > b. do write in those blocks
> > > c. atomically update extent mapping.
> > >
> > > In this, threads wanting to atomic write to the same address will use the
> > > new blocks and can trample over one another before we atomically update the
> > > mapping.
> > So iiuc, w/ software fallback, a thread atomically writing to a range
> > will use a new block A. Another parallel thread trying to atomically
> > write to the same range will also use A, and there is no serialization
> > b/w the 2 so A could end up with a mix of data from both threads.
>
> right
>
> >
> > If this is true, aren't we violating the atomic guarantees. Nothing
> > prevents the userspace from doing overlapping parallel atomic writes and
> > it is kernels duty to error out if the write could get torn.
>
> Correct, but simply userspace should not do this. Direct I/O applications
> are responsible for ordering.
>
> We guarantee that the write is committed all-or-nothing, but do rely on
> userspace not issuing racing atomic writes or racing regular writes.
>
> I can easily change this, as I mentioned, but I am not convinced that it is
> a must.

Purely from a design point of view, I feel we are breaking atomicity and
hence we should serialize or just stop userspace from doing this (which
is a bit extreme).

I know userspace should ideally not do overwriting atomic writes but if
it is something we are allowing (which we do) then it is
kernel's responsibility to ensure atomicity. Sure we can penalize them
by serializing the writes but not by tearing it.

With that reasoning, I don't think the test should accomodate for this
particular scenario.

Regards,
ojaswin