linux-kernel - Re: [PATCH 01/10] rcu: Directly lock rdp->nocb

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]

Message-ID: <20200604163655.GC29598@paulmck-ThinkPad-P72>
Date:   Thu, 4 Jun 2020 09:36:55 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Frederic Weisbecker <frederic@...nel.org>
Cc:     Joel Fernandes <joel@...lfernandes.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Josh Triplett <josh@...htriplett.org>
Subject: Re: [PATCH 01/10] rcu: Directly lock rdp->nocb_lock on nocb code
 entrypoints

On Thu, Jun 04, 2020 at 01:41:22PM +0200, Frederic Weisbecker wrote:
> On Fri, May 22, 2020 at 10:57:39AM -0700, Paul E. McKenney wrote:
> > On Wed, May 20, 2020 at 08:29:49AM -0400, Joel Fernandes wrote:
> > > Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> > 
> > Thank you for looking this over, Joel!
> > 
> > Is it feasible to make rcu_nocb_lock*() and rcu_nocb_unlock*() "do the
> > right thing", even when things are changing?  If it is feasible, that
> > would prevent any number of "interesting" copy-pasta and "just now became
> > common code" bugs down the road.
> 
> This won't be pretty:
> 
>     locked = rcu_nocb_lock();
>     rcu_nocb_unlock(locked);

I was thinking in terms of a bit in the rcu_data structure into
which rcu_nocb_lock() and friends stored the status, and from which
rcu_nocb_unlock() and friends retrieved that same status.  Sort of like
how preemptible RCU uses the ->rcu_read_lock_nesting field in task_struct.

As noted, this does require reworking the hotplug code to avoid the
current holding of two such locks concurrently, which I am happy to do
if that helps.

Or am I missing a subtle (or not-so-subtle) twist here?

> And anyway we still want to unconditionally lock on many places,
> regardless of the offloaded state. I don't know how we could have
> a magic helper do the unconditional lock on some places and the
> conditional on others.

I was assuming (perhaps incorrectly) that an intermediate phase between
not-offloaded and offloaded would take care of all of those cases.

> Also the point of turning the lock helpers into primitives is to make
> it clearer as to where we really need unconditional locking and where
> we allow it to be conditional. A gift to reviewers :-)

Unless and until someone does a copy-pasta, thus unconditionally
doing the wrong thing.  ;-)

If we cannot avoid different spellings of ->cblist in different places,
such is life, but I do want to make sure that we have fully considered
the alternatives.

						Thanx, Paul