linux-kernel - Re: [PATCH] Remove GP_REPLAY state from rcu

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: <20191007140942.GA12213@redhat.com>
Date:   Mon, 7 Oct 2019 16:09:42 +0200
From:   Oleg Nesterov <oleg@...hat.com>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     linux-kernel@...r.kernel.org, bristot@...hat.com,
        peterz@...radead.org, paulmck@...nel.org, rcu@...r.kernel.org,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH] Remove GP_REPLAY state from rcu_sync

On 10/04, Joel Fernandes wrote:
>
> On Fri, Oct 04, 2019 at 05:41:03PM +0200, Oleg Nesterov wrote:
> > On 10/04, Joel Fernandes (Google) wrote:
> > >
> > > Taking a step back, why did we intend to have
> > > to wait for a new GP if another rcu_sync_exit() comes while one is still
> > > in progress?
> >
> > To ensure that if another CPU sees rcu_sync_is_idle() (GP_IDLE) after you
> > do rcu_sync_exit(), then it must also see all memory changes you did before
> > rcu_sync_exit().
>
> Would this not be better implemented using memory barriers, than starting new
> grace periods just for memory ordering? A memory barrier is lighter than
> having to go through a grace period. So something like: if the state is
> already GP_EXIT, then rcu_sync_exit() issues a memory barrier instead of
> replaying. But if state is GP_PASSED, then wait for a grace period.

But these 2 cases do not differ. If we can use mb() if GP_EXIT, then we can
do the same if state == GP_PASSED and just move the state to GP_IDLE, and
remove both GP_PASSED/GP_REPLAY states.

However, in this case the readers will need the barrier too, and rcu_sync_enter()
will _always_ need to block (wait for GP).

rcu_sync.c is "equivalent" to the following implementation:


            struct rcu_sync_struct {
                    atomic_t writers;
            };

            bool rcu_sync_is_idle(rss)
            {
                    return atomic_read(rss->writers) == 0;
            }

            void rcu_sync_enter(rss)
            {
                    atomic_inc(rss->writers);
                    synchronize_rcu();
            }

            void rcu_sync_exit(rss)
            {
                    synchronize_rcu();
                    atomic_dec(rss->writers);
            }

except

	- rcu_sync_exit() never blocks

	- synchronize_rcu/call_rci is called only if it is really needed.
	  In particular, if 2 writers come in a row the 2nd one will not
	  block in _enter().

Oleg.