linux-kernel - Re: [PATCH 1/5] workqueue: wq_pool_mutex protects the attrs-installation

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: <20160311175013.GJ24046@htj.duckdns.org>
Date:	Fri, 11 Mar 2016 12:50:13 -0500
From:	Tejun Heo <tj@...nel.org>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Lai Jiangshan <laijs@...fujitsu.com>, linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: [PATCH 1/5] workqueue: wq_pool_mutex protects the
 attrs-installation

Hello, Steven.

On Thu, Mar 10, 2016 at 04:44:11PM -0500, Steven Rostedt wrote:
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index a3915ab..fa8b949 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -127,6 +127,12 @@ enum {
> >   *
> >   * PR: wq_pool_mutex protected for writes.  Sched-RCU protected for reads.
> >   *
> > + * PW: wq_pool_mutex and wq->mutex protected for writes.  Any one of them
> > + *     protected for reads.
> > + *
> > + * PWR: wq_pool_mutex and wq->mutex protected for writes. Any one of them
> > + *      or sched-RCU for reads.
> 
> How exactly is sched-RCU protecting this here? The cause for the 4.1-rt issue
> is that we converted the local_irq_save() in queue_work_on() into a
> "local_lock_irqsave()" which when PREEMPT_RT is enabled will be a mutex that
> disables migration (can not migrate). This also prevents the current CPU from
> going offline.
> 
> Does this code really need to be protected from being preempted, or is
> disabling migration good enough?

That's used for workqueue->numa_pwq_tbl[] and it's derefed while
determining the pwq to use for the node.  The queueing path already
has local irq disabled and pwqs are sched rcu protected.  If the task
gets preempted inbetween, sched rcu grace period might trigger and
queueing path might try to deref an already free pwq, so yeah, I think
it does need preemption protection there.

Thanks.

-- 
tejun