linux-kernel - Re: [PATCH 7/7] RCU, workqueue: Implement 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]

Date:   Tue, 6 Mar 2018 10:30:29 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Tejun Heo <tj@...nel.org>
Cc:     Jann Horn <jannh@...gle.com>,
        Paul McKenney <paulmck@...ux.vnet.ibm.com>,
        Benjamin LaHaise <bcrl@...ck.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Kent Overstreet <kent.overstreet@...il.com>,
        security@...nel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kernel-team <kernel-team@...com>
Subject: Re: [PATCH 7/7] RCU, workqueue: Implement rcu_work

On Tue, Mar 6, 2018 at 9:33 AM, Tejun Heo <tj@...nel.org> wrote:
>
> This patch introduces rcu_work, a workqueue work variant which gets
> executed after a RCU grace period, and converts the open coded
> bouncing in fs/aio and kernel/cgroup.

So I like the concept, but I have two comments:

 - can we split this patch up, so that if somebody bisects a problem
to it, we'll see if it's cgroup or aio that triggers it?

 - this feels wrong:

> +struct rcu_work {
> +       struct work_struct work;
> +       struct rcu_head rcu;
> +
> +       /* target workqueue and CPU ->rcu uses to queue ->work */
> +       struct workqueue_struct *wq;
> +       int cpu;
> +};

That "int cpu" really doesn't feel like it makes sense for an
rcu_work. The rcu_call() part fundamentally will happen on any CPU,
and sure, it could then schedule the work on something else, but that
doesn't sound like a particularly sound interface.

So I'd like to either just make the thing always just use
WORK_CPU_UNBOUND, or hear some kind of (handwaving ok) explanation for
why something else would ever make sense. If the action is
fundamentally delayed by RCU, why would it make a difference which CPU
it runs on?

One reason for that is that I get this feeling that the multiple
stages of waiting *might* be unnecessary. Are there no situations
where a "rcu_work" might just end up devolving to be just a regular
work? Or maybe situations where the rcu callback is done in process
context, and the work can just be done immediately? I'm a tiny bit
worried about queueing artifacts, where we end up having tons of
resources in flight.

But this really is just a "this feels wrong". I have no real hard
technical reason.

                Linus