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

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: <1342804767.2583.40.camel@twins>
Date:	Fri, 20 Jul 2012 19:19:27 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Tejun Heo <tj@...nel.org>
Cc:	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	tglx@...utronix.de, linux-pm@...r.kernel.org
Subject: Re: [PATCHSET] workqueue: reimplement CPU hotplug to keep idle
 workers

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);

  ....
}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/