From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753262AbbE0VFK (ORCPT ); Wed, 27 May 2015 17:05:10 -0400 Received: from e38.co.us.ibm.com ([32.97.110.159]:54061 "EHLO e38.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753038AbbE0VEy (ORCPT ); Wed, 27 May 2015 17:04:54 -0400 Date: Wed, 27 May 2015 14:04:46 -0700 From: "Paul E. McKenney" To: "Nicholas A. Bellinger" Cc: Bart Van Assche , "Nicholas A. Bellinger" , target-devel , linux-scsi , linux-kernel , Christoph Hellwig , Hannes Reinecke , Sagi Grimberg Subject: Re: [PATCH-v2 2/4] target: Drop lun_sep_lock for se_lun->lun_se_dev RCU usage Message-ID: <20150527210446.GP5989@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <1432623445-25776-1-git-send-email-nab@daterainc.com> <1432623445-25776-3-git-send-email-nab@daterainc.com> <55648374.80802@sandisk.com> <1432704585.26863.47.camel@haakon3.risingtidesystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1432704585.26863.47.camel@haakon3.risingtidesystems.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15052721-0029-0000-0000-00000A1C68F3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 26, 2015 at 10:29:45PM -0700, Nicholas A. Bellinger wrote: > On Tue, 2015-05-26 at 16:30 +0200, Bart Van Assche wrote: > > On 05/26/15 08:57, Nicholas A. Bellinger wrote: > > > @@ -625,6 +626,7 @@ int core_dev_add_initiator_node_lun_acl( > > > u32 lun_access) > > > { > > > struct se_node_acl *nacl = lacl->se_lun_nacl; > > > + struct se_device *dev = lockless_dereference(lun->lun_se_dev); > > > > > > if (!nacl) > > > return -EINVAL; > > > > An attempt to run this code on a system with RCU debugging enabled > > resulted in the following complaint: > > > > =============================== > > [ INFO: suspicious RCU usage. ] > > 4.1.0-rc1-lio-dbg+ #1 Not tainted > > ------------------------------- > > drivers/target/target_core_device.c:617 suspicious rcu_dereference_check() usage! > > > > other info that might help us debug this: > > > > > > rcu_scheduler_active = 1, debug_locks = 1 > > 2 locks held by ln/1497: > > #0: (sb_writers#11){.+.+.+}, at: [] mnt_want_write+0x24/0x50 > > #1: (&sb->s_type->i_mutex_key#14/1){+.+.+.}, at: [] filename_create+0xad/0x1a0 > > > > stack backtrace: > > CPU: 0 PID: 1497 Comm: ln Not tainted 4.1.0-rc1-lio-dbg+ #1 > > Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 > > 0000000000000001 ffff88005955bd68 ffffffff814fa346 0000000000000011 > > ffff880058bf1270 ffff88005955bd98 ffffffff810ab235 ffff880050db9a68 > > ffff880058ae2e68 0000000000000002 ffff880058ae4120 ffff88005955be08 > > Call Trace: > > [] dump_stack+0x4f/0x7b > > [] lockdep_rcu_suspicious+0xd5/0x110 > > [] core_dev_add_initiator_node_lun_acl+0xec/0x190 [target_core_mod] > > [] ? get_parent_ip+0x11/0x50 > > [] target_fabric_mappedlun_link+0x129/0x240 [target_core_mod] > > [] ? target_fabric_mappedlun_link+0x9c/0x240 [target_core_mod] > > [] configfs_symlink+0x13d/0x360 [configfs] > > [] vfs_symlink+0x58/0xb0 > > [] SyS_symlink+0x65/0xc0 > > [] system_call_fastpath+0x16/0x7a > > > > In this particular case, the se_device behind se_lun->lun_se_dev > __rcu protected pointer can't be released without first releasing the > pre-existing se_lun->lun_group reference to se_device->dev_group. > > And since se_lun->lun_group is the source of a configfs symlink to > se_lun_acl->se_lun_group here, the se_lun associated RCU pointer and > underlying se_device can't be released out from under the above > target_fabric_mappedlun_link() code accessing a __rcu protected pointer. > > Paul, is lockless_dereference the correct notation for this type of > use-case..? My guess is "no", but I don't claim to understand your use case. The splat is against some other code than the patch, judging by the patch line numbers. The rule is that if a pointer points to something that is freed (or reused) after a grace period, you mark that pointer with __rcu. Any access to that pointer must then be accessed in an RCU read-side critical section, using one of the RCU list iterators or one of the rcu_dereference() macros. No lockless_dereference() in this case. You use lockless_dereference() when something other than RCU controls when the pointer target is freed. Thanx, Paul