Re: [PATCH] xfs: mark the i_delayed_blks access in xfs_file_release as racy

[Dave Chinner <david@fromorbit.com>]

On Tue, May 13, 2025 at 07:26:14AM +0200, Christoph Hellwig wrote:
> We don't bother with the ILOCK as this is best-effort and thus a racy
> access is ok.  Add a data_race() annotation to make that clear to
> memory model verifiers.

IMO, that's the thin edge of a wedge. There are dozens of places in
XFS where we check variable values without holding the lock needed
to serialise the read against modification.

For example, i_delayed_blks updates are protected by the ILOCK, so
there's a data race if we read it without the ILOCK held.  We do
this in:

	xfs_file_release() - the one this patch addresses
	xfs_getbmap() - unlocked access for data fork
	xfs_can_free_eofblocks() - checked twice without locking
	xfs_inodegc_set_reclaimable() - unlocked, but debug
	xfs_inode_has_filedata() - unlocked via xfs_inactive()
	xfs_vn_getattr() - unlocked
	xfs_qm_dqusage_adjust() - unlocked ASSERT

And that's just this one variable in the inode - there are lots of
others we check without bothering to lock the inode.

e.g. pretty much everythign that xfs_vn_getattr() reads is a data
race because "unlocked racy access" is the way stat is designed to
work on Linux.

Hence my question - are we now going to make it policy that every
possible racy access must be marked with data_race() because there
is some new bot that someone is running that will complain if we
don't?  Are you committing to playing whack-a-mole with the memory
model verifiers to silence all the false positives from these
known-to-be-safe access patterns?

-Dave.
-- 
Dave Chinner
david@fromorbit.com