Re: [PATCH v2] xfs: use xfs_trans_ail_copy_lsn for lockless li_lsn read in CIL formatting

[Dave Chinner <dgc@kernel.org>]

On Tue, Mar 24, 2026 at 11:22:42PM -0700, Christoph Hellwig wrote:
> Looks good to me, even if the additional lock on 32-bit might hurt
> that one person or two running performance critical workloads on
> 32-bit systems:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> But I'd really like to have Dave look over this as he's the resident
> expert in this area.

Seen it, haven't had time to look at it in depth. Complex.

In general, updates to lip->li_lsn at AIL insert time do not hold
object locks (buffer locks, dquot locks, inode locks, etc) and so
can race with reading of lip->li_lsn at any other time.  On 64 bit
platforms, this isn't an issue - but on 32 bit platforms any of
these LSN reads could tear if lip->li_lsn is updated at the same
time.

However, I think -all- objects are pinned pinned in memory when
lip->li_lsn is updated in the xlog_cil_ail_insert() path. Hence I
suspect the race condition of concurrent update/read is much more
limited that it possibly could be because being pinned while
updating rules out all the buffer writeback path verifier reads
from racing with writes.

I suspect that inode and dquot relogging are one possible vector.
inode and dquot flushing are another (e.g. xfs_inode_to_disk() in
xfs_iflush() looks suspect), and then it gets complex...

... because we also read lip->li_lsn in various IO completion
routines.  There's the possibility that objects are relogged whilst
IO is in progress (inodes, yes, dquots maybe, buffers no) and so
reads in IO completion processing could also possibly race with
journal completion modifying the lip->li_lsn....

I really don't have time audit all this code, but I a in a quick
scan I do see several locations where there could potentially be
similar torn 64 bit reads and writes...

-Dave.
-- 
Dave Chinner
dgc@kernel.org