google / kernel-sanitizers

Linux Kernel Sanitizers, fast bug-detectors for the Linux kernel
https://google.github.io/kernel-sanitizers/
442 stars 87 forks source link

Annotate more synchronization primitives #202

Open dvyukov opened 9 years ago

dvyukov commented 9 years ago

We need to annotate:

include/linux/percpu-rwsem.h / kernel/locking/percpu-rwsem.c

Absence of annotation does not lead to false positives, but adds parasitic synchronization (readers don't synchronize on rwlocks).

xairy commented 9 years ago

This is not easy, since percpu_rwsem_down/up_read/write call a lot of functions that can enter scheduler including down_write and down_read. We could enable events back around those calls (same as we did with rwsem), but in that case we won't ignore synchronization from down_write and down_read.

xairy commented 9 years ago

Hm, I see false positives in tests with percpu-rwsem (without any additional annotations).

386 struct percpu_rw_semaphore pcrws_sync;                                          
387                                                                                 
388 static void pcrws_main(void *arg)                                               
389 {                                                                               
390         int rv = percpu_init_rwsem(&pcrws_sync);                                
391         BUG_ON(rv != 0);                                                        
392 }                                                                               
393                                                                                 
394 static void pcrws_write_write(void *arg)                                        
395 {                                                                               
396         percpu_down_write(&pcrws_sync);                                         
397         *((int *)arg) = 1;                                                      
398         percpu_up_write(&pcrws_sync);                                           
399 }                                                                               
400                                                                                 
401 static void pcrws_read_read(void *arg)                                          
402 {                                                                               
403         percpu_down_read(&pcrws_sync);                                          
404         *((int *)arg + 4) = *((int *)arg);                                      
405         percpu_up_read(&pcrws_sync);                                            
406 } 
==================================================================
ThreadSanitizer: data-race in pcrws_write_write

Write at 0xffff88028a19b400 of size 4 by thread 2838 on CPU 0:
 [<ffffffff81252c92>] pcrws_write_write+0x22/0x40 mm/ktsan/tests_inst.c:397
 [<ffffffff812523fd>] thr_func+0x2d/0x50 mm/ktsan/tests_inst.c:33
 [<ffffffff810b98c1>] kthread+0x161/0x180 kernel/kthread.c:209
 [<ffffffff81ea8c8f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:529

Previous read at 0xffff88028a19b400 of size 4 by thread 2839 on CPU 1:
 [<ffffffff81252c47>] pcrws_read_read+0x27/0x50 mm/ktsan/tests_inst.c:404
 [<ffffffff812523fd>] thr_func+0x2d/0x50 mm/ktsan/tests_inst.c:33
 [<ffffffff810b98c1>] kthread+0x161/0x180 kernel/kthread.c:209
 [<ffffffff81ea8c8f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:529

Mutexes locked by thread 2838:
Mutex 408151 is locked here:
 [<ffffffff81ea5a95>] down_write+0x65/0x80 kernel/locking/rwsem.c:62
 [<ffffffff810dfaf5>] percpu_down_write+0x45/0x190 kernel/locking/percpu-rwsem.c:145
 [<ffffffff81252c8a>] pcrws_write_write+0x1a/0x40 mm/ktsan/tests_inst.c:396
 [<ffffffff812523fd>] thr_func+0x2d/0x50 mm/ktsan/tests_inst.c:33
 [<ffffffff810b98c1>] kthread+0x161/0x180 kernel/kthread.c:209
 [<ffffffff81ea8c8f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:529

Mutexes locked by thread 2839:
Mutex 408151 is read locked here:
 [<ffffffff81ea5a15>] down_read+0x45/0x60 kernel/locking/rwsem.c:28
 [<ffffffff810df9ea>] percpu_down_read+0x6a/0xa0 kernel/locking/percpu-rwsem.c:85
 [<ffffffff81252c3f>] pcrws_read_read+0x1f/0x50 mm/ktsan/tests_inst.c:403
 [<ffffffff812523fd>] thr_func+0x2d/0x50 mm/ktsan/tests_inst.c:33
 [<ffffffff810b98c1>] kthread+0x161/0x180 kernel/kthread.c:209
 [<ffffffff81ea8c8f>] ret_from_fork+0x3f/0x70 arch/x86/entry/entry_64.S:529
==================================================================
dvyukov commented 9 years ago

Another reason to properly annotate them ;)