Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756850AbbEVLbg (ORCPT ); Fri, 22 May 2015 07:31:36 -0400 Received: from verein.lst.de ([213.95.11.211]:33197 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756692AbbEVLbV (ORCPT ); Fri, 22 May 2015 07:31:21 -0400 Date: Fri, 22 May 2015 13:31:19 +0200 From: Christoph Hellwig To: "Nicholas A. Bellinger" Cc: Christoph Hellwig , "Nicholas A. Bellinger" , target-devel , linux-scsi , linux-kernel , Hannes Reinecke , Christoph Hellwig , Sagi Grimberg , "Paul E. McKenney" Subject: Re: [PATCH-v2 1/9] target: Convert se_node_acl->device_list[] to RCU hlist Message-ID: <20150522113119.GA28758@lst.de> References: <1432275071-28882-1-git-send-email-nab@daterainc.com> <1432275071-28882-2-git-send-email-nab@daterainc.com> <20150522082411.GB5384@infradead.org> <1432284930.898.19.camel@haakon3.risingtidesystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1432284930.898.19.camel@haakon3.risingtidesystems.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2113 Lines: 51 On Fri, May 22, 2015 at 01:55:30AM -0700, Nicholas A. Bellinger wrote: > > This update will now be racy, ditto for the read/write_bytes update > > later. > > This should become an atomic_long_t increment, yes..? Yes. > Yes, this helper is from your patch above. > > Considering there is a single user of it here, and complexities involved > for a RCU conversion + bisect, is it really work adding as a separate > patch ahead of this one..? The golden Linus style is to put preparatory patches first so that the actual logic change is as small as possible. Adding helpers so that low level accesses that will e changed soon is a very typical case for that. > > > + kref_put(&orig->pr_kref, target_pr_kref_release); > > > + wait_for_completion(&orig->pr_comp); > > > > > > > > + kref_put(&orig->pr_kref, target_pr_kref_release); > > > /* > > > - * Disable struct se_dev_entry LUN ACL mapping > > > + * Before fireing off RCU callback, wait for any in process SPEC_I_PT=1 > > > + * or REGISTER_AND_MOVE PR operation to complete. > > > */ > > > + wait_for_completion(&orig->pr_comp); > > > + kfree_rcu(orig, rcu_head); > > > > The release callback should just call kfree_rcu, no need to wait for the > > release in the caller. > > > > Why doesn't se_dev_entry release this need to wait for the special case > references to drop..? Why would it? There is no access to the structure at this point, so there is no point to keep it around localy. If there were other references to it they by defintion don't need it anymore by the time they drop the reference count. Freeing a structure as soon as the refcount drops zero is the normal style all over the place. Waiting for a reference count only makes sense if it's a drain style operation where you don't free the structure but you just want to wait for some class of consumers to stop using it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/