linux-kernel - Re: [PATCH 2/3] task: RCU protect tasks on the runqueue

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: <CAHk-=wjVGLr8wArT9P4MXxA-XpkG=9ZXdjM3vpemSF25vYiLoA@mail.gmail.com>
Date:   Tue, 3 Sep 2019 10:08:21 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Chris Metcalf <cmetcalf@...hip.com>,
        Christoph Lameter <cl@...ux.com>,
        Kirill Tkhai <tkhai@...dex.ru>, Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        Davidlohr Bueso <dave@...olabs.net>
Subject: Re: [PATCH 2/3] task: RCU protect tasks on the runqueue

On Tue, Sep 3, 2019 at 9:45 AM Eric W. Biederman <ebiederm@...ssion.com> wrote:
>
> So with a big fat comment explaining why it is safe we could potentially
> use RCU_INIT_POINTER.  I currently don't see where the appropriate
> barriers are so I can not write that comment or with a clear conscious
> write the code to use RCU_INIT_POINTER instead of rcu_assign_pointer.

The only difference ends up being that RCU_INIT_POINTER() is just a
store, while rcu_assign_pointer() uses a smp_store_release().

(There is some build-time special case code to make
rcu_assign_pointer(NULL) avoid the store_release, but that is
irrelevant for this discussion).

So from a memory ordering standpoint,
RCU_INIT_POINTER-vs-rcu_assign_pointer doesn't change what pointer you
get (on the other CPU that does the reading), but only whether the
stores to behind the pointer have been ordered wrt the reading too.

Which no existing case can care about, since it didn't use to have any
ordering anyway before this patch series. The individual values read
off the thread pointer had their own individual memory ordering rules
(ie instead of making the _pointer_ be the serialization point, we
have rules for how "p->on_cpu" is ordered wrt the rq lock etc).

So one argument for just using RCU_INIT_POINTER is that it's the same
ordering that we had before, and then it's up to any users of that
pointer to order any accesses to any fields in 'struct task_struct'.

Conversely, one argument for using rcu_assign_pointer() is that when
we pair it with an RCU read, we get certain ordering guarantees
automatically. So _if_ we have fields that change when a process is
put on the run-queue, and the RCU users want to read those fields,
then the release/acquire semantics might perform better than potential
existing smp memory barriers we might have right now.

                 Linus