On Sun, Jul 31, 2022 at 11:48:32PM +0100, Matthew Wilcox wrote:
> On Sun, Jul 31, 2022 at 10:30:11AM -0700, Paul E. McKenney wrote:
> > That said, I confess that I am having a hard time finding the
> > buffer_locked() definition. So if buffer_locked() uses normal C-language
> > accesses to sample the BH_Lock bit of the ->b_state field, then yes,
> > there could be a problem. The compiler would then be free to reorder
> > calls to buffer_locked() because it could then assume that no other
> > thread was touching that ->b_state field.
>
> You're having a hard time finding it because it's constructed with the C
> preprocessor. I really wish we generated header files using CPP once
> and then included the generated/ file. That would make them greppable.
>
> You're looking for include/linux/buffer_head.h and it's done like this:
>
> enum bh_state_bits {
> ...
> BH_Lock, /* Is locked */
> ...
>
> #define BUFFER_FNS(bit, name) \
> ...
> static __always_inline int buffer_##name(const struct buffer_head *bh) \
> { \
> return test_bit(BH_##bit, &(bh)->b_state); \
> }
>
> BUFFER_FNS(Lock, locked)
>
> (fwiw, the page/folio versions of these functions don't autogenerate
> the lock or uptodate ones because they need extra functions called)
Thank you!
Another thing that would have helped me find it would have been to leave
the "BH_" prefix on the bit name in the BUFFER_FNS() invocation, as in
ditch the "BH_##" in the definition and then just say "BUFFER_FNS(BH_Lock,
locked)".
But what is life without a challenge? ;-/
Anyway, on x86 this will use the constant_test_bit() function, which
uses a volatile declaration for its parameter. So it should avoid
vulnerability to the vicissitudes of the compiler.
So I feel much better now.
Thanx, Paul
