Messages in this thread | | | From | Linus Torvalds <> | Date | Tue, 6 Mar 2018 10:30:29 -0800 | Subject | Re: [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
| |