linux-kernel - Re: [PATCH] doc/rcuref: Document real world examples in kernel

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] [day] [month] [year] [list]

Message-ID: <20190329044629.GA234754@google.com>
Date:   Fri, 29 Mar 2019 00:46:29 -0400
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Jann Horn <jannh@...gle.com>
Cc:     kernel list <linux-kernel@...r.kernel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        linux-doc@...r.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH] doc/rcuref: Document real world examples in kernel

On Fri, Mar 29, 2019 at 12:44:05AM -0400, Joel Fernandes wrote:
> On Fri, Mar 29, 2019 at 05:06:21AM +0100, Jann Horn wrote:
> > On Fri, Mar 29, 2019 at 3:40 AM Joel Fernandes (Google)
> > <joel@...lfernandes.org> wrote:
> > > Document similar real world examples in the kernel corresponding to the
> > > second and third code snippets. Also correct an issue in
> > > release_referenced() in the code snippet example.
> > >
> > > Cc: oleg@...hat.com
> > > Cc: jannh@...gle.com
> > > Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> > >
> > > ---
> > >  Documentation/RCU/rcuref.txt | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/RCU/rcuref.txt b/Documentation/RCU/rcuref.txt
> > > index 613033ff2b9b..e5f4a49f886a 100644
> > > --- a/Documentation/RCU/rcuref.txt
> > > +++ b/Documentation/RCU/rcuref.txt
> > > @@ -28,7 +28,8 @@ add()                         search_and_reference()
> > >  release_referenced()                   delete()
> > >  {                                      {
> > >      ...                                            write_lock(&list_lock);
> > > -    atomic_dec(&el->rc, relfunc)           ...
> > > +    if(atomic_dec_and_test(&el->rc))       ...
> > > +       kfree(el);
> > >      ...                                            remove_element
> > >  }                                          write_unlock(&list_lock);
> > >                                             ...
> > > @@ -114,6 +115,11 @@ element can therefore safely be freed.  This in turn guarantees that if
> > >  any reader finds the element, that reader may safely acquire a reference
> > >  without checking the value of the reference counter.
> > >
> > > +The other advantage of the last pattern is, if there are several calls to
> > > +search_and_reference() in parallel to the delete(), then all of those will
> > > +succeed in obtaining a reference to the object if the object could be found in
> > > +the list before it was deleted in delete().
> > 
> > Isn't this the same as what the previous paragraph said? "if
> > any reader finds the element, that reader may safely acquire a reference
> > without checking the value of the reference counter".
> 
> You are right.  But I felt it was less explicit about the fact that several
> search_and_reference() calls can succeed will not FAIL like the previous example.
> 
> I can reword it as below:
> 
> As can be seen, a clear advantage of the last pattern is, if there are
> several calls to search_and_reference() in parallel to the delete(), then all
> of those will succeed in obtaining a reference to the object if the object
> could be found in the list before it was deleted in delete(), unlike the
> previous pattern which would fail to acquire references.
> 
> Or, can I entirely drop it if Paul and others also feel it is not necessary.

Here I meant "I can entirely drop this part of the patch if Paul and others
also feel it is not necessary."

thanks,

 - Joel