Re: [PATCHSET] workqueue: reimplement CPU hotplug to keep idle workers

[Peter Zijlstra <peterz@infradead.org>]

On Fri, 2012-07-20 at 10:08 -0700, Tejun Heo wrote:
> Hello, Peter.
> 
> On Fri, Jul 20, 2012 at 07:01:40PM +0200, Peter Zijlstra wrote:
> > On Fri, 2012-07-20 at 09:52 -0700, Tejun Heo wrote:
> > > Ooh, yeah, I agree.  That's next on the wq to-do list.  The problem is
> > > that queue_work() is implemented in terms of queue_work_on().  
> > 
> > But that's trivial to fix, both could use __queue_work() without too
> > much bother, right?
> 
> Hmmm?  Not really.  We need to keep count of the ones which reqested
> fixed binding - ie. the ones which explicitly used queue_work_on() -
> and then flush on wq CPU_DOWN.  Then, we need to audit the current
> users which are using queue_work{_on}() + explicit FLUSH on CPU_DOWN
> and convert them.

No, that's the wrong way about. Just add another WQ_flag, so you can
tell which work-queues want sane semantics and those mucking about for
laughs, say WQ_NON_AFFINE.

Then those with sane semantics (!WQ_NON_AFFINE) get an explicit flush on
DOWN and don't need to muck about with detaching and re-attaching etc..

> > > The assumption was that they should flush during CPU_DOWN but it
> > > probably will be much better to require users which need CPU affinity
> > > to always use queue_work_on() - instead of implicit local affinity
> > > from queue_work() - and flush them automatically from wq callback.
> > 
> > Right, and when you create this new mode, which you need to know to
> > flush on DOWN, you can simply put a BUG_ON in queue_work_on() when this
> > mode is set.
> 
> BUG_ON() on queue_work_on()?  Do you mean if the target CPU is down?
> If so, yeah, I'd probably go with WARN_ON_ONCE() but we should whine
> on it.

No, when you're trying to enqueue something to a specific cpu but don't
provide strict per-cpu semantics.

int queue_work_on(int cpu, struct workqueue_struct *wq, 
		  struct work_struct *work)
{
  WARN_ON(wq->flags & WQ_NON_AFFINE);

  ....
}