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

[Christoph Hellwig <hch@lst.de>]

On Wed, May 14, 2025 at 07:37:14AM +1000, Dave Chinner wrote:
> 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.

Yes. And the linux kernel memory consistency model ask us to mark them,
see tools/memory-model/Documentation/access-marking.txt.

This fails painful at first, but I'd actually wish we'd have tools
enforcing this as strongly as possible as developers (well me at least)
seem to think a racy access is just fine more often than they should, and
needing an annotation and a comment is a pretty good way to sure that.

> 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?

It's not really a "new bot".  It has been official memory consistency
policy for a while, but it just hasn't been well enforced.  For new code
asking if the review is racy and needs a marking or use READ_ONCE() and
WRITE_ONCE() has been part of the usual review protocol.  Reviewing old
code and fixing things we got wrong will take a while, but I'm actually
glad about more bots for that.