Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 10 Jan 2020 22:40:33 +0530
From:   Amol Grover <frextrite@gmail.com>
To:     Keith Busch <kbusch@kernel.org>
Cc:     Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
        Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>,
        linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-kernel-mentees@lists.linuxfoundation.org,
        Joel Fernandes <joel@joelfernandes.org>,
        Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>,
        "Paul E . McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH] drivers: nvme: target: core: Pass lockdep expression to
 RCU lists
Message-ID: <20200110171033.GA23748@workstation-kernel-dev>
References: <20200110132356.27110-1-frextrite@gmail.com>
 <20200110163134.GA18579@redsun51.ssa.fujisawa.hgst.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200110163134.GA18579@redsun51.ssa.fujisawa.hgst.com>
User-Agent: Mutt/1.12.2 (2019-09-21)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Jan 11, 2020 at 01:31:34AM +0900, Keith Busch wrote:
> On Fri, Jan 10, 2020 at 06:53:58PM +0530, Amol Grover wrote:
> > +#define subsys_lock_held() \
> > +	lockdep_is_held(&subsys->lock)
> 
> This macro requires "struct nvmet_subsys *subsys" was previously declared
> in the function using it, which isn't obvious when looking at the users. I
> don't think that's worth the conciseness.
> 

Hey Keith,
If I understand correctly, you're saying `*subsys` is always declared in the
function using it, right? I too think, this could cause confusion to the person
going through the code, I'll fix it right away.

Thanks
Amol

> > @@ -555,7 +558,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
> >  	} else {
> >  		struct nvmet_ns *old;
> >  
> > -		list_for_each_entry_rcu(old, &subsys->namespaces, dev_link) {
> > +		list_for_each_entry_rcu(old, &subsys->namespaces, dev_link,
> > +							subsys_lock_held()) {
> >  			BUG_ON(ns->nsid == old->nsid);
> >  			if (ns->nsid < old->nsid)
> >  				break;