1*4882a593Smuzhiyun.. SPDX-License-Identifier: GPL-2.0 2*4882a593Smuzhiyun 3*4882a593Smuzhiyun================= 4*4882a593SmuzhiyunLockdep-RCU Splat 5*4882a593Smuzhiyun================= 6*4882a593Smuzhiyun 7*4882a593SmuzhiyunLockdep-RCU was added to the Linux kernel in early 2010 8*4882a593Smuzhiyun(http://lwn.net/Articles/371986/). This facility checks for some common 9*4882a593Smuzhiyunmisuses of the RCU API, most notably using one of the rcu_dereference() 10*4882a593Smuzhiyunfamily to access an RCU-protected pointer without the proper protection. 11*4882a593SmuzhiyunWhen such misuse is detected, an lockdep-RCU splat is emitted. 12*4882a593Smuzhiyun 13*4882a593SmuzhiyunThe usual cause of a lockdep-RCU slat is someone accessing an 14*4882a593SmuzhiyunRCU-protected data structure without either (1) being in the right kind of 15*4882a593SmuzhiyunRCU read-side critical section or (2) holding the right update-side lock. 16*4882a593SmuzhiyunThis problem can therefore be serious: it might result in random memory 17*4882a593Smuzhiyunoverwriting or worse. There can of course be false positives, this 18*4882a593Smuzhiyunbeing the real world and all that. 19*4882a593Smuzhiyun 20*4882a593SmuzhiyunSo let's look at an example RCU lockdep splat from 3.0-rc5, one that 21*4882a593Smuzhiyunhas long since been fixed:: 22*4882a593Smuzhiyun 23*4882a593Smuzhiyun ============================= 24*4882a593Smuzhiyun WARNING: suspicious RCU usage 25*4882a593Smuzhiyun ----------------------------- 26*4882a593Smuzhiyun block/cfq-iosched.c:2776 suspicious rcu_dereference_protected() usage! 27*4882a593Smuzhiyun 28*4882a593Smuzhiyunother info that might help us debug this:: 29*4882a593Smuzhiyun 30*4882a593Smuzhiyun rcu_scheduler_active = 1, debug_locks = 0 31*4882a593Smuzhiyun 3 locks held by scsi_scan_6/1552: 32*4882a593Smuzhiyun #0: (&shost->scan_mutex){+.+.}, at: [<ffffffff8145efca>] 33*4882a593Smuzhiyun scsi_scan_host_selected+0x5a/0x150 34*4882a593Smuzhiyun #1: (&eq->sysfs_lock){+.+.}, at: [<ffffffff812a5032>] 35*4882a593Smuzhiyun elevator_exit+0x22/0x60 36*4882a593Smuzhiyun #2: (&(&q->__queue_lock)->rlock){-.-.}, at: [<ffffffff812b6233>] 37*4882a593Smuzhiyun cfq_exit_queue+0x43/0x190 38*4882a593Smuzhiyun 39*4882a593Smuzhiyun stack backtrace: 40*4882a593Smuzhiyun Pid: 1552, comm: scsi_scan_6 Not tainted 3.0.0-rc5 #17 41*4882a593Smuzhiyun Call Trace: 42*4882a593Smuzhiyun [<ffffffff810abb9b>] lockdep_rcu_dereference+0xbb/0xc0 43*4882a593Smuzhiyun [<ffffffff812b6139>] __cfq_exit_single_io_context+0xe9/0x120 44*4882a593Smuzhiyun [<ffffffff812b626c>] cfq_exit_queue+0x7c/0x190 45*4882a593Smuzhiyun [<ffffffff812a5046>] elevator_exit+0x36/0x60 46*4882a593Smuzhiyun [<ffffffff812a802a>] blk_cleanup_queue+0x4a/0x60 47*4882a593Smuzhiyun [<ffffffff8145cc09>] scsi_free_queue+0x9/0x10 48*4882a593Smuzhiyun [<ffffffff81460944>] __scsi_remove_device+0x84/0xd0 49*4882a593Smuzhiyun [<ffffffff8145dca3>] scsi_probe_and_add_lun+0x353/0xb10 50*4882a593Smuzhiyun [<ffffffff817da069>] ? error_exit+0x29/0xb0 51*4882a593Smuzhiyun [<ffffffff817d98ed>] ? _raw_spin_unlock_irqrestore+0x3d/0x80 52*4882a593Smuzhiyun [<ffffffff8145e722>] __scsi_scan_target+0x112/0x680 53*4882a593Smuzhiyun [<ffffffff812c690d>] ? trace_hardirqs_off_thunk+0x3a/0x3c 54*4882a593Smuzhiyun [<ffffffff817da069>] ? error_exit+0x29/0xb0 55*4882a593Smuzhiyun [<ffffffff812bcc60>] ? kobject_del+0x40/0x40 56*4882a593Smuzhiyun [<ffffffff8145ed16>] scsi_scan_channel+0x86/0xb0 57*4882a593Smuzhiyun [<ffffffff8145f0b0>] scsi_scan_host_selected+0x140/0x150 58*4882a593Smuzhiyun [<ffffffff8145f149>] do_scsi_scan_host+0x89/0x90 59*4882a593Smuzhiyun [<ffffffff8145f170>] do_scan_async+0x20/0x160 60*4882a593Smuzhiyun [<ffffffff8145f150>] ? do_scsi_scan_host+0x90/0x90 61*4882a593Smuzhiyun [<ffffffff810975b6>] kthread+0xa6/0xb0 62*4882a593Smuzhiyun [<ffffffff817db154>] kernel_thread_helper+0x4/0x10 63*4882a593Smuzhiyun [<ffffffff81066430>] ? finish_task_switch+0x80/0x110 64*4882a593Smuzhiyun [<ffffffff817d9c04>] ? retint_restore_args+0xe/0xe 65*4882a593Smuzhiyun [<ffffffff81097510>] ? __kthread_init_worker+0x70/0x70 66*4882a593Smuzhiyun [<ffffffff817db150>] ? gs_change+0xb/0xb 67*4882a593Smuzhiyun 68*4882a593SmuzhiyunLine 2776 of block/cfq-iosched.c in v3.0-rc5 is as follows:: 69*4882a593Smuzhiyun 70*4882a593Smuzhiyun if (rcu_dereference(ioc->ioc_data) == cic) { 71*4882a593Smuzhiyun 72*4882a593SmuzhiyunThis form says that it must be in a plain vanilla RCU read-side critical 73*4882a593Smuzhiyunsection, but the "other info" list above shows that this is not the 74*4882a593Smuzhiyuncase. Instead, we hold three locks, one of which might be RCU related. 75*4882a593SmuzhiyunAnd maybe that lock really does protect this reference. If so, the fix 76*4882a593Smuzhiyunis to inform RCU, perhaps by changing __cfq_exit_single_io_context() to 77*4882a593Smuzhiyuntake the struct request_queue "q" from cfq_exit_queue() as an argument, 78*4882a593Smuzhiyunwhich would permit us to invoke rcu_dereference_protected as follows:: 79*4882a593Smuzhiyun 80*4882a593Smuzhiyun if (rcu_dereference_protected(ioc->ioc_data, 81*4882a593Smuzhiyun lockdep_is_held(&q->queue_lock)) == cic) { 82*4882a593Smuzhiyun 83*4882a593SmuzhiyunWith this change, there would be no lockdep-RCU splat emitted if this 84*4882a593Smuzhiyuncode was invoked either from within an RCU read-side critical section 85*4882a593Smuzhiyunor with the ->queue_lock held. In particular, this would have suppressed 86*4882a593Smuzhiyunthe above lockdep-RCU splat because ->queue_lock is held (see #2 in the 87*4882a593Smuzhiyunlist above). 88*4882a593Smuzhiyun 89*4882a593SmuzhiyunOn the other hand, perhaps we really do need an RCU read-side critical 90*4882a593Smuzhiyunsection. In this case, the critical section must span the use of the 91*4882a593Smuzhiyunreturn value from rcu_dereference(), or at least until there is some 92*4882a593Smuzhiyunreference count incremented or some such. One way to handle this is to 93*4882a593Smuzhiyunadd rcu_read_lock() and rcu_read_unlock() as follows:: 94*4882a593Smuzhiyun 95*4882a593Smuzhiyun rcu_read_lock(); 96*4882a593Smuzhiyun if (rcu_dereference(ioc->ioc_data) == cic) { 97*4882a593Smuzhiyun spin_lock(&ioc->lock); 98*4882a593Smuzhiyun rcu_assign_pointer(ioc->ioc_data, NULL); 99*4882a593Smuzhiyun spin_unlock(&ioc->lock); 100*4882a593Smuzhiyun } 101*4882a593Smuzhiyun rcu_read_unlock(); 102*4882a593Smuzhiyun 103*4882a593SmuzhiyunWith this change, the rcu_dereference() is always within an RCU 104*4882a593Smuzhiyunread-side critical section, which again would have suppressed the 105*4882a593Smuzhiyunabove lockdep-RCU splat. 106*4882a593Smuzhiyun 107*4882a593SmuzhiyunBut in this particular case, we don't actually dereference the pointer 108*4882a593Smuzhiyunreturned from rcu_dereference(). Instead, that pointer is just compared 109*4882a593Smuzhiyunto the cic pointer, which means that the rcu_dereference() can be replaced 110*4882a593Smuzhiyunby rcu_access_pointer() as follows:: 111*4882a593Smuzhiyun 112*4882a593Smuzhiyun if (rcu_access_pointer(ioc->ioc_data) == cic) { 113*4882a593Smuzhiyun 114*4882a593SmuzhiyunBecause it is legal to invoke rcu_access_pointer() without protection, 115*4882a593Smuzhiyunthis change would also suppress the above lockdep-RCU splat. 116