On Thu, Aug 25, 2022 at 05:03:40PM -0400, Mikulas Patocka wrote:
> From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
>
> There are several places in the kernel where wait_on_bit is not followed
> by a memory barrier (for example, in drivers/md/dm-bufio.c:new_read). On
> architectures with weak memory ordering, it may happen that memory
> accesses that follow wait_on_bit are reordered before wait_on_bit and they
> may return invalid data.
>
> Fix this class of bugs by introducing a new function "test_bit_acquire"
> that works like test_bit, but has acquire memory ordering semantics.
>
> Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
...
> --- linux-2.6.orig/include/asm-generic/bitops/generic-non-atomic.h
> +++ linux-2.6/include/asm-generic/bitops/generic-non-atomic.h
> @@ -4,6 +4,7 @@
> #define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H
>
> #include <linux/bits.h>
> +#include <asm/barrier.h>
>
> #ifndef _LINUX_BITOPS_H
> #error only <linux/bitops.h> can be included directly
> @@ -127,6 +128,18 @@ generic_test_bit(unsigned long nr, const
> return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
> }
>
> +/**
> + * generic_test_bit - Determine whether a bit is set with acquire semantics
Trivial: Name in the kerneldoc isn't the same as the actual function name.
(Also, "with acquire semantics" is supposed to modify "Determine", not
"is set" -- we don't set bits using acquire semantics. You could change
this to "Determine, with acquire semantics, whether a bit is set".)
Alan Stern
> + * @nr: bit number to test
> + * @addr: Address to start counting from
> + */
> +static __always_inline bool
> +generic_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr)
> +{
> + unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
> + return 1UL & (smp_load_acquire(p) >> (nr & (BITS_PER_LONG-1)));
> +}
