Re: [PATCH] xfs: annotate lockless b_flags read in xfs_buf_lock

[Cen Zhang]

Hi Dave,

> No. READ_ONCE should not be used to annotate a benign data access
> race. data_race() should be used because all it does is turn off
> KASAN for that access, and unlike READ_ONCE(), there is no code
> change when KASAN is not enabled.

Thank you for the review.  You're right -- data_race() is the correct
annotation for a known-benign race.  READ_ONCE() adds an unnecessary
compiler barrier and, without a paired WRITE_ONCE() on the write side,
is not the right pattern here.

> But, in reality, the race condition here is more than just the
> b_flags access.  There is a big comment above the function explaining what
> the check does, and that the buffer pinned check that precedes the
> b_flags check can race with journal completion, too.

Agreed.  The atomic_read() on b_pin_count is already KCSAN-safe, so
only the b_flags access needs a data_race() annotation to suppress
the KCSAN report.  The commit message in v2 now acknowledges that the
entire pre-semaphore check is racy by design.

I'll send a v2 using data_race().

Thanks,
Cen