On Sun, Mar 30, 2014 at 08:57:51AM -0400, Tejun Heo wrote:
> On Thu, Mar 27, 2014 at 06:21:00PM +0100, Frederic Weisbecker wrote:
> > The workqueues are all listed in a global list protected by a big mutex.
> > And this big mutex is used in apply_workqueue_attrs() as well.
> >
> > Now as we plan to implement a directory to control the cpumask of
> > all non-ABI unbound workqueues, we want to be able to iterate over all
> > unbound workqueues and call apply_workqueue_attrs() for each of
> > them with the new cpumask.
> >
> > But the risk for a deadlock is on the way: we need to iterate the list
> > of workqueues under wq_pool_mutex. But then apply_workqueue_attrs()
> > itself calls wq_pool_mutex.
>
> Wouldn't the right thing to do would be factoring out
> apply_workqueue_attrs_locked()? It's cleaner to block out addition of
> new workqueues while the masks are being updated anyway.
I'm not quite sure I get what you suggest. Do you mean have apply_workqueue_attrs_locked()
calling apply_workqueue_attrs() under the lock on this patch?
Thanks.
Hello,
On Thu, Apr 03, 2014 at 04:48:28PM +0200, Frederic Weisbecker wrote:
> > Wouldn't the right thing to do would be factoring out
> > apply_workqueue_attrs_locked()? It's cleaner to block out addition of
> > new workqueues while the masks are being updated anyway.
>
> I'm not quite sure I get what you suggest. Do you mean have
> apply_workqueue_attrs_locked() calling apply_workqueue_attrs() under
> the lock on this patch?
Not sure it still matters but I was suggesting that creating
apply_workqueue_attrs_locked() which requires the caller to handle
locking and making apply_workqueue_attrs() a wrapper which grabs and
releases lock around it, and using the former in locked iteration
would work. lol has this explanation made it any clearer or is it
even worse now? :)
Thanks.
--
tejun
On Thu, Apr 03, 2014 at 11:01:28AM -0400, Tejun Heo wrote:
> Hello,
>
> On Thu, Apr 03, 2014 at 04:48:28PM +0200, Frederic Weisbecker wrote:
> > > Wouldn't the right thing to do would be factoring out
> > > apply_workqueue_attrs_locked()? It's cleaner to block out addition of
> > > new workqueues while the masks are being updated anyway.
> >
> > I'm not quite sure I get what you suggest. Do you mean have
> > apply_workqueue_attrs_locked() calling apply_workqueue_attrs() under
> > the lock on this patch?
>
> Not sure it still matters but I was suggesting that creating
> apply_workqueue_attrs_locked() which requires the caller to handle
> locking and making apply_workqueue_attrs() a wrapper which grabs and
> releases lock around it, and using the former in locked iteration
> would work. lol has this explanation made it any clearer or is it
> even worse now? :)
I see, it gets a little better now :)
Maybe it still matters because I still need to iterate over unbound
workqueues to apply an update on "cpu_unbound_wqs_mask". And the list must remain
stable while I call apply_workqueue_attrs() on workqueues.
Anyway, we'll see how it looks like :)