linux-kernel - Re: [RFC][PATCH 07/10] orangefs: Use RCU for destroy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 26 Feb 2017 19:34:01 -0500
From:   Mike Marshall <hubcap@...ibond.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Chris Mason <clm@...com>, LKML <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        David Howells <dhowells@...hat.com>, elena.reshetova@...el.com,
        Hans Liljestrand <ishkamiel@...il.com>,
        David Windsor <dwindsor@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [RFC][PATCH 07/10] orangefs: Use RCU for destroy_inode

Since Orangefs uses ref-walk, not rcu-walk, this patch with call_rcu
has seemed weird to me.

Using the git log, I searched back to where it seems to me call_rcu was
added, a giant patch from 2005 by David Howells which includes tons of
source and a large amount of documentation.

It seems that the call back function in call_rcu is used to destroy objects
only after readers are known to be finished, the mechanism by which
the readers are known to be finished is described in the documentation.

Perhaps I shouldn't think that Orangefs doesn't use rcu-walk, rather
that it switches to ref-walk from rcu-walk when d_revalidate returns
ECHILD (which it does right away).

-Mike

On Sat, Feb 25, 2017 at 3:31 PM, Mike Marshall <hubcap@...ibond.com> wrote:
> After looking through the code and seeing how some other filesystems
> use call_rcu, it seems that call_rcu has to do with consistency and
> waiting for stuff to complete before returning an object to the slab cache,
> whereas we were just calling kmem_cache_free without worrying about that
> kind of stuff...
>
> Is that a "close enough" description of the error that is being
> fixed here?
>
> -Mike
>
> On Fri, Feb 24, 2017 at 6:00 PM, Mike Marshall <hubcap@...ibond.com> wrote:
>> Thanks Al... I was going to try and evaluate that patch next
>> week, now all I have to do is test it <g> ...
>>
>> -Mike
>>
>> On Fri, Feb 24, 2017 at 3:52 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
>>> That, AFAICS, fixes a real bug.  Applied, and it needs Cc:stable as well.
>>>
>>>
>>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
>>>> ---
>>>>  fs/orangefs/super.c |    9 ++++++++-
>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> --- a/fs/orangefs/super.c
>>>> +++ b/fs/orangefs/super.c
>>>> @@ -115,6 +115,13 @@ static struct inode *orangefs_alloc_inod
>>>>       return &orangefs_inode->vfs_inode;
>>>>  }
>>>>
>>>> +static void orangefs_i_callback(struct rcu_head *head)
>>>> +{
>>>> +     struct inode *inode = container_of(head, struct inode, i_rcu);
>>>> +     struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>>>> +     kmem_cache_free(orangefs_inode_cache, orangefs_inode);
>>>> +}
>>>> +
>>>>  static void orangefs_destroy_inode(struct inode *inode)
>>>>  {
>>>>       struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
>>>> @@ -123,7 +130,7 @@ static void orangefs_destroy_inode(struc
>>>>                       "%s: deallocated %p destroying inode %pU\n",
>>>>                       __func__, orangefs_inode, get_khandle_from_ino(inode));
>>>>
>>>> -     kmem_cache_free(orangefs_inode_cache, orangefs_inode);
>>>> +     call_rcu(&inode->i_rcu, orangefs_i_callback);
>>>>  }
>>>>
>>>>  /*
>>>>
>>>>