lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH] locking/osq_lock: fix a data race in osq_wait_next
From
Date


> On Jan 22, 2020, at 5:38 PM, Marco Elver <elver@google.com> wrote:
>
>
>
> On Wed, 22 Jan 2020, Qian Cai wrote:
>
>>
>>
>>> On Jan 22, 2020, at 11:59 AM, Will Deacon <will@kernel.org> wrote:
>>>
>>> I don't understand this; 'next' is a local variable.
>>>
>>> Not keen on the onslaught of random "add a READ_ONCE() to shut the
>>> sanitiser up" patches we're going to get from kcsan :(
>>
>> My fault. I suspect it is node->next. I’ll do a bit more testing to confirm.
>
> If possible, decode and get the line numbers. I have observed a data

[ 667.817131] Reported by Kernel Concurrency Sanitizer on:
[ 667.823200] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W L 5.5.0-rc7-next-20200121+ #9
[ 667.832839] Hardware name: HPE ProLiant DL385 Gen10/ProLiant DL385 Gen10, BIOS A40 07/10/2019
[ 667.842132] ==================================================================
[ 672.299421] ==================================================================
[ 672.307449] BUG: KCSAN: data-race in osq_lock / osq_lock

[ 672.315741] write (marked) to 0xffff8f613013be00 of 8 bytes by task 971 on cpu 59:
[ 672.324085] osq_lock+0x2fb/0x340 kernel/locking/osq_lock.c:200
[ 672.328149] __mutex_lock+0x277/0xd20 kernel/locking/mutex.c:657
[ 672.332561] mutex_lock_nested+0x31/0x40 kernel/locking/mutex.c:1118
[ 672.337236] memcg_create_kmem_cache+0x2e/0x190 mm/slab_common.c:659
[ 672.342534] memcg_kmem_cache_create_func+0x40/0x80
[ 672.348177] process_one_work+0x54c/0xbe0
[ 672.352940] worker_thread+0x80/0x650
[ 672.357351] kthread+0x1e0/0x200
[ 672.361324] ret_from_fork+0x27/0x50

[ 672.367875] read to 0xffff8f613013be00 of 8 bytes by task 708 on cpu 50:
[ 672.375345] osq_lock+0x234/0x340 kernel/locking/osq_lock.c:78
[ 672.379431] __mutex_lock+0x277/0xd20 kernel/locking/mutex.c:657
[ 672.383862] mutex_lock_nested+0x31/0x40 kernel/locking/mutex.c:1118
[ 672.388537] memcg_create_kmem_cache+0x2e/0x190 mm/slab_common.c:659
[ 672.393824] memcg_kmem_cache_create_func+0x40/0x80
[ 672.399461] process_one_work+0x54c/0xbe0
[ 672.404229] worker_thread+0x80/0x650
[ 672.408640] kthread+0x1e0/0x200
[ 672.412613] ret_from_fork+0x27/0x50

This?

diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
index 1f7734949ac8..832e87966dcf 100644
--- a/kernel/locking/osq_lock.c
+++ b/kernel/locking/osq_lock.c
@@ -75,7 +75,7 @@ osq_wait_next(struct optimistic_spin_queue *lock,
* wait for either @lock to point to us, through its Step-B, or
* wait for a new @node->next from its Step-C.
*/
- if (node->next) {
+ if (READ_ONCE(node->next)) {
next = xchg(&node->next, NULL);
if (next)
break;
> race in osq_lock before, however, this is the only one I have recently
> seen in osq_lock:
>
> read to 0xffff88812c12d3d4 of 4 bytes by task 23304 on cpu 0:
> osq_lock+0x170/0x2f0 kernel/locking/osq_lock.c:143
>
> while (!READ_ONCE(node->locked)) {
> /*
> * If we need to reschedule bail... so we can block.
> * Use vcpu_is_preempted() to avoid waiting for a preempted
> * lock holder:
> */
> --> if (need_resched() || vcpu_is_preempted(node_cpu(node->prev)))
> goto unqueue;
>
> cpu_relax();
> }
>
> where
>
> static inline int node_cpu(struct optimistic_spin_node *node)
> {
> --> return node->cpu - 1;
> }
>
>
> write to 0xffff88812c12d3d4 of 4 bytes by task 23334 on cpu 1:
> osq_lock+0x89/0x2f0 kernel/locking/osq_lock.c:99
>
> bool osq_lock(struct optimistic_spin_queue *lock)
> {
> struct optimistic_spin_node *node = this_cpu_ptr(&osq_node);
> struct optimistic_spin_node *prev, *next;
> int curr = encode_cpu(smp_processor_id());
> int old;
>
> node->locked = 0;
> node->next = NULL;
> --> node->cpu = curr;
>
>
> Thanks,
> -- Marco

\
 
 \ /
  Last update: 2020-01-23 00:55    [W:0.071 / U:0.460 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site