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@xxxxxxxxxxxxx
