lkml.org 
[lkml]   [2015]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH-v2 1/9] target: Convert se_node_acl->device_list[] to RCU hlist
From
Date
On Fri, 2015-05-22 at 13:31 +0200, Christoph Hellwig wrote:
> 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.

Converted.

>
> > 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.
>

That would be applicable here, if the patch in question had anything to
do with the actual RCU conversion itself.

Since it doesn't, I'll keep it as a separate patch after the RCU
specific changes, along with the other improvements.

> > > > + 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?

It originally had to wait in order for the se_dev_entry consumers to
drop, before it could be reused in se_node_acl->device_list[]..

> 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.

.. but since it's not being reused anymore, doing a kfree_rcu() from the
final kref_put() should be fine.

Converting now.



\
 
 \ /
  Last update: 2015-05-26 00:41    [W:0.557 / U:0.316 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site