Re: [PATCH v2] make buffer_locked provide an acquire semantics

["Paul E. McKenney" <paulmck@kernel.org>]

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