Commit 8238b457 authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Linus Torvalds

wait_on_bit: add an acquire memory barrier

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: default avatarMikulas Patocka <mpatocka@redhat.com>
Acked-by: default avatarWill Deacon <will@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 4c612826
...@@ -58,13 +58,11 @@ Like with atomic_t, the rule of thumb is: ...@@ -58,13 +58,11 @@ Like with atomic_t, the rule of thumb is:
- RMW operations that have a return value are fully ordered. - RMW operations that have a return value are fully ordered.
- RMW operations that are conditional are unordered on FAILURE, - RMW operations that are conditional are fully ordered.
otherwise the above rules apply. In the case of test_and_set_bit_lock(),
if the bit in memory is unchanged by the operation then it is deemed to have
failed.
Except for a successful test_and_set_bit_lock() which has ACQUIRE semantics and Except for a successful test_and_set_bit_lock() which has ACQUIRE semantics,
clear_bit_unlock() which has RELEASE semantics. clear_bit_unlock() which has RELEASE semantics and test_bit_acquire which has
ACQUIRE semantics.
Since a platform only has a single means of achieving atomic operations Since a platform only has a single means of achieving atomic operations
the same barriers as for atomic_t are used, see atomic_t.txt. the same barriers as for atomic_t are used, see atomic_t.txt.
......
...@@ -207,6 +207,20 @@ static __always_inline bool constant_test_bit(long nr, const volatile unsigned l ...@@ -207,6 +207,20 @@ static __always_inline bool constant_test_bit(long nr, const volatile unsigned l
(addr[nr >> _BITOPS_LONG_SHIFT])) != 0; (addr[nr >> _BITOPS_LONG_SHIFT])) != 0;
} }
static __always_inline bool constant_test_bit_acquire(long nr, const volatile unsigned long *addr)
{
bool oldbit;
asm volatile("testb %2,%1"
CC_SET(nz)
: CC_OUT(nz) (oldbit)
: "m" (((unsigned char *)addr)[nr >> 3]),
"i" (1 << (nr & 7))
:"memory");
return oldbit;
}
static __always_inline bool variable_test_bit(long nr, volatile const unsigned long *addr) static __always_inline bool variable_test_bit(long nr, volatile const unsigned long *addr)
{ {
bool oldbit; bool oldbit;
...@@ -226,6 +240,13 @@ arch_test_bit(unsigned long nr, const volatile unsigned long *addr) ...@@ -226,6 +240,13 @@ arch_test_bit(unsigned long nr, const volatile unsigned long *addr)
variable_test_bit(nr, addr); variable_test_bit(nr, addr);
} }
static __always_inline bool
arch_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr)
{
return __builtin_constant_p(nr) ? constant_test_bit_acquire(nr, addr) :
variable_test_bit(nr, addr);
}
/** /**
* __ffs - find first set bit in word * __ffs - find first set bit in word
* @word: The word to search * @word: The word to search
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H #define __ASM_GENERIC_BITOPS_GENERIC_NON_ATOMIC_H
#include <linux/bits.h> #include <linux/bits.h>
#include <asm/barrier.h>
#ifndef _LINUX_BITOPS_H #ifndef _LINUX_BITOPS_H
#error only <linux/bitops.h> can be included directly #error only <linux/bitops.h> can be included directly
...@@ -127,6 +128,18 @@ generic_test_bit(unsigned long nr, const volatile unsigned long *addr) ...@@ -127,6 +128,18 @@ generic_test_bit(unsigned long nr, const volatile unsigned long *addr)
return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
} }
/**
* generic_test_bit_acquire - Determine, with acquire semantics, whether a bit is set
* @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)));
}
/* /*
* const_*() definitions provide good compile-time optimizations when * const_*() definitions provide good compile-time optimizations when
* the passed arguments can be resolved at compile time. * the passed arguments can be resolved at compile time.
...@@ -137,6 +150,7 @@ generic_test_bit(unsigned long nr, const volatile unsigned long *addr) ...@@ -137,6 +150,7 @@ generic_test_bit(unsigned long nr, const volatile unsigned long *addr)
#define const___test_and_set_bit generic___test_and_set_bit #define const___test_and_set_bit generic___test_and_set_bit
#define const___test_and_clear_bit generic___test_and_clear_bit #define const___test_and_clear_bit generic___test_and_clear_bit
#define const___test_and_change_bit generic___test_and_change_bit #define const___test_and_change_bit generic___test_and_change_bit
#define const_test_bit_acquire generic_test_bit_acquire
/** /**
* const_test_bit - Determine whether a bit is set * const_test_bit - Determine whether a bit is set
......
...@@ -142,4 +142,16 @@ _test_bit(unsigned long nr, const volatile unsigned long *addr) ...@@ -142,4 +142,16 @@ _test_bit(unsigned long nr, const volatile unsigned long *addr)
return arch_test_bit(nr, addr); return arch_test_bit(nr, addr);
} }
/**
* _test_bit_acquire - Determine, with acquire semantics, whether a bit is set
* @nr: bit number to test
* @addr: Address to start counting from
*/
static __always_inline bool
_test_bit_acquire(unsigned long nr, const volatile unsigned long *addr)
{
instrument_atomic_read(addr + BIT_WORD(nr), sizeof(long));
return arch_test_bit_acquire(nr, addr);
}
#endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H */ #endif /* _ASM_GENERIC_BITOPS_INSTRUMENTED_NON_ATOMIC_H */
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#define arch___test_and_change_bit generic___test_and_change_bit #define arch___test_and_change_bit generic___test_and_change_bit
#define arch_test_bit generic_test_bit #define arch_test_bit generic_test_bit
#define arch_test_bit_acquire generic_test_bit_acquire
#include <asm-generic/bitops/non-instrumented-non-atomic.h> #include <asm-generic/bitops/non-instrumented-non-atomic.h>
......
...@@ -12,5 +12,6 @@ ...@@ -12,5 +12,6 @@
#define ___test_and_change_bit arch___test_and_change_bit #define ___test_and_change_bit arch___test_and_change_bit
#define _test_bit arch_test_bit #define _test_bit arch_test_bit
#define _test_bit_acquire arch_test_bit_acquire
#endif /* __ASM_GENERIC_BITOPS_NON_INSTRUMENTED_NON_ATOMIC_H */ #endif /* __ASM_GENERIC_BITOPS_NON_INSTRUMENTED_NON_ATOMIC_H */
...@@ -59,6 +59,7 @@ extern unsigned long __sw_hweight64(__u64 w); ...@@ -59,6 +59,7 @@ extern unsigned long __sw_hweight64(__u64 w);
#define __test_and_clear_bit(nr, addr) bitop(___test_and_clear_bit, nr, addr) #define __test_and_clear_bit(nr, addr) bitop(___test_and_clear_bit, nr, addr)
#define __test_and_change_bit(nr, addr) bitop(___test_and_change_bit, nr, addr) #define __test_and_change_bit(nr, addr) bitop(___test_and_change_bit, nr, addr)
#define test_bit(nr, addr) bitop(_test_bit, nr, addr) #define test_bit(nr, addr) bitop(_test_bit, nr, addr)
#define test_bit_acquire(nr, addr) bitop(_test_bit_acquire, nr, addr)
/* /*
* Include this here because some architectures need generic_ffs/fls in * Include this here because some architectures need generic_ffs/fls in
......
...@@ -156,7 +156,7 @@ static __always_inline int buffer_uptodate(const struct buffer_head *bh) ...@@ -156,7 +156,7 @@ static __always_inline int buffer_uptodate(const struct buffer_head *bh)
* make it consistent with folio_test_uptodate * make it consistent with folio_test_uptodate
* pairs with smp_mb__before_atomic in set_buffer_uptodate * pairs with smp_mb__before_atomic in set_buffer_uptodate
*/ */
return (smp_load_acquire(&bh->b_state) & (1UL << BH_Uptodate)) != 0; return test_bit_acquire(BH_Uptodate, &bh->b_state);
} }
#define bh_offset(bh) ((unsigned long)(bh)->b_data & ~PAGE_MASK) #define bh_offset(bh) ((unsigned long)(bh)->b_data & ~PAGE_MASK)
......
...@@ -71,7 +71,7 @@ static inline int ...@@ -71,7 +71,7 @@ static inline int
wait_on_bit(unsigned long *word, int bit, unsigned mode) wait_on_bit(unsigned long *word, int bit, unsigned mode)
{ {
might_sleep(); might_sleep();
if (!test_bit(bit, word)) if (!test_bit_acquire(bit, word))
return 0; return 0;
return out_of_line_wait_on_bit(word, bit, return out_of_line_wait_on_bit(word, bit,
bit_wait, bit_wait,
...@@ -96,7 +96,7 @@ static inline int ...@@ -96,7 +96,7 @@ static inline int
wait_on_bit_io(unsigned long *word, int bit, unsigned mode) wait_on_bit_io(unsigned long *word, int bit, unsigned mode)
{ {
might_sleep(); might_sleep();
if (!test_bit(bit, word)) if (!test_bit_acquire(bit, word))
return 0; return 0;
return out_of_line_wait_on_bit(word, bit, return out_of_line_wait_on_bit(word, bit,
bit_wait_io, bit_wait_io,
...@@ -123,7 +123,7 @@ wait_on_bit_timeout(unsigned long *word, int bit, unsigned mode, ...@@ -123,7 +123,7 @@ wait_on_bit_timeout(unsigned long *word, int bit, unsigned mode,
unsigned long timeout) unsigned long timeout)
{ {
might_sleep(); might_sleep();
if (!test_bit(bit, word)) if (!test_bit_acquire(bit, word))
return 0; return 0;
return out_of_line_wait_on_bit_timeout(word, bit, return out_of_line_wait_on_bit_timeout(word, bit,
bit_wait_timeout, bit_wait_timeout,
...@@ -151,7 +151,7 @@ wait_on_bit_action(unsigned long *word, int bit, wait_bit_action_f *action, ...@@ -151,7 +151,7 @@ wait_on_bit_action(unsigned long *word, int bit, wait_bit_action_f *action,
unsigned mode) unsigned mode)
{ {
might_sleep(); might_sleep();
if (!test_bit(bit, word)) if (!test_bit_acquire(bit, word))
return 0; return 0;
return out_of_line_wait_on_bit(word, bit, action, mode); return out_of_line_wait_on_bit(word, bit, action, mode);
} }
......
...@@ -47,7 +47,7 @@ __wait_on_bit(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_ ...@@ -47,7 +47,7 @@ __wait_on_bit(struct wait_queue_head *wq_head, struct wait_bit_queue_entry *wbq_
prepare_to_wait(wq_head, &wbq_entry->wq_entry, mode); prepare_to_wait(wq_head, &wbq_entry->wq_entry, mode);
if (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags)) if (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags))
ret = (*action)(&wbq_entry->key, mode); ret = (*action)(&wbq_entry->key, mode);
} while (test_bit(wbq_entry->key.bit_nr, wbq_entry->key.flags) && !ret); } while (test_bit_acquire(wbq_entry->key.bit_nr, wbq_entry->key.flags) && !ret);
finish_wait(wq_head, &wbq_entry->wq_entry); finish_wait(wq_head, &wbq_entry->wq_entry);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment