lkml.org 
[lkml]   [2018]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 7/7] RCU, workqueue: Implement rcu_work
On Tue, Mar 6, 2018 at 9:33 AM, Tejun Heo <tj@kernel.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

\
 
 \ /
  Last update: 2018-03-06 19:31    [W:0.136 / U:1.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site