[PATCH 1/2] workqueue: clear POOL_DISASSOCIATED in rebind_workers()

[PATCH 1/2] workqueue: clear POOL_DISASSOCIATED in rebind_workers()

[Lai Jiangshan]

The commit a9ab775bcadf ("workqueue: directly restore CPU affinity of workers
from CPU_ONLINE") moved the pool->lock into rebind_workers() without also
moving "pool->flags &= ~POOL_DISASSOCIATED" into.

There is no wrong that "pool->flags &= ~POOL_DISASSOCIATED" is kept outside,
but there is no benefit either. We move it into rebind_workers() and
achieve these benefits:
  1) better readability, POOL_DISASSOCIATED is cleared in rebind_workers()
     as supposed.
  2) when POOL_DISASSOCIATED is cleared, we can ensure that the running
     workers of the pool are on the local CPU (pool->cpu).

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 kernel/workqueue.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 61381a2..5ae491e 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -4535,6 +4535,7 @@ static void rebind_workers(struct worker_pool *pool)
 						  pool->attrs->cpumask) < 0);
 
 	spin_lock_irq(&pool->lock);
+	pool->flags &= ~POOL_DISASSOCIATED;
 
 	for_each_pool_worker(worker, pool) {
 		unsigned int worker_flags = worker->flags;
@@ -4637,10 +4638,6 @@ static int workqueue_cpu_up_callback(struct notifier_block *nfb,
 			mutex_lock(&pool->attach_mutex);
 
 			if (pool->cpu == cpu) {
-				spin_lock_irq(&pool->lock);
-				pool->flags &= ~POOL_DISASSOCIATED;
-				spin_unlock_irq(&pool->lock);
-
 				rebind_workers(pool);
 			} else if (pool->cpu < 0) {
 				restore_unbound_workers_cpumask(pool, cpu);
-- 
1.7.4.4

[PATCH 2/2] workqueue: stronger test in process_one_work()

[Lai Jiangshan]

When POOL_DISASSOCIATED is cleared, the running worker's local CPU should
be the same as pool->cpu without any exception even during cpu-hotplug.

This fix changes "(proposition_A && proposition_B && proposition_C)"
to "(proposition_B && proposition_C)", so if the old compound proposition
is true, the new one must be true too. so this fix will not hide any
possible bug which can be hit by old test.

CC: Jason J. Herne <jjherne@linux.vnet.ibm.com>
CC: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 kernel/workqueue.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5ae491e..204dd95 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2015,13 +2015,8 @@ __acquires(&pool->lock)
 
 	lockdep_copy_map(&lockdep_map, &work->lockdep_map);
 #endif
-	/*
-	 * Ensure we're on the correct CPU.  DISASSOCIATED test is
-	 * necessary to avoid spurious warnings from rescuers servicing the
-	 * unbound or a disassociated pool.
-	 */
-	WARN_ON_ONCE(!(worker->flags & WORKER_UNBOUND) &&
-		     !(pool->flags & POOL_DISASSOCIATED) &&
+	/* Ensure we're on the correct CPU. */
+	WARN_ON_ONCE(!(pool->flags & POOL_DISASSOCIATED) &&
 		     raw_smp_processor_id() != pool->cpu);
 
 	/*
-- 
1.7.4.4

Re: [PATCH 2/2] workqueue: stronger test in process_one_work()

[Tejun Heo]

On Tue, Jun 03, 2014 at 03:33:28PM +0800, Lai Jiangshan wrote:
> When POOL_DISASSOCIATED is cleared, the running worker's local CPU should
> be the same as pool->cpu without any exception even during cpu-hotplug.
> 
> This fix changes "(proposition_A && proposition_B && proposition_C)"
> to "(proposition_B && proposition_C)", so if the old compound proposition
> is true, the new one must be true too. so this fix will not hide any
> possible bug which can be hit by old test.
> 
> CC: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> CC: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Applied to wq/for-3.17 with minor updates.

Nice set of cleanups.  Thanks!

-- 
tejun

Re: [PATCH 2/2] workqueue: stronger test in process_one_work()

[Lai Jiangshan]

On 06/20/2014 03:44 AM, Tejun Heo wrote:
> On Tue, Jun 03, 2014 at 03:33:28PM +0800, Lai Jiangshan wrote:
>> When POOL_DISASSOCIATED is cleared, the running worker's local CPU should
>> be the same as pool->cpu without any exception even during cpu-hotplug.
>>
>> This fix changes "(proposition_A && proposition_B && proposition_C)"
>> to "(proposition_B && proposition_C)", so if the old compound proposition
>> is true, the new one must be true too. so this fix will not hide any
>> possible bug which can be hit by old test.
>>
>> CC: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> CC: Sasha Levin <sasha.levin@oracle.com>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
> Applied to wq/for-3.17 with minor updates.
> 
> Nice set of cleanups.  Thanks!
> 

Hi,Tejun

I found the slight earlier 6 patches are in wq/for-3.17.
But these two patches (in this email thread) are not in wq/for-3.17 yet.

workqueue: clear POOL_DISASSOCIATED in rebind_workers()
workqueue: stronger test in process_one_work()

Thanks,
Lai

Re: [PATCH 2/2] workqueue: stronger test in process_one_work()

[Lai Jiangshan]

Ping.

Thanks,
Lai

On 06/26/2014 07:27 PM, Lai Jiangshan wrote:
> On 06/20/2014 03:44 AM, Tejun Heo wrote:
>> On Tue, Jun 03, 2014 at 03:33:28PM +0800, Lai Jiangshan wrote:
>>> When POOL_DISASSOCIATED is cleared, the running worker's local CPU should
>>> be the same as pool->cpu without any exception even during cpu-hotplug.
>>>
>>> This fix changes "(proposition_A && proposition_B && proposition_C)"
>>> to "(proposition_B && proposition_C)", so if the old compound proposition
>>> is true, the new one must be true too. so this fix will not hide any
>>> possible bug which can be hit by old test.
>>>
>>> CC: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>>> CC: Sasha Levin <sasha.levin@oracle.com>
>>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>>
>> Applied to wq/for-3.17 with minor updates.
>>
>> Nice set of cleanups.  Thanks!
>>
> 
> Hi,Tejun
> 
> I found the slight earlier 6 patches are in wq/for-3.17.
> But these two patches (in this email thread) are not in wq/for-3.17 yet.
> 
> workqueue: clear POOL_DISASSOCIATED in rebind_workers()
> workqueue: stronger test in process_one_work()
> 
> Thanks,
> Lai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH 2/2] workqueue: stronger test in process_one_work()

[Tejun Heo]

On Tue, Jun 03, 2014 at 03:33:28PM +0800, Lai Jiangshan wrote:
> When POOL_DISASSOCIATED is cleared, the running worker's local CPU should
> be the same as pool->cpu without any exception even during cpu-hotplug.
> 
> This fix changes "(proposition_A && proposition_B && proposition_C)"
> to "(proposition_B && proposition_C)", so if the old compound proposition
> is true, the new one must be true too. so this fix will not hide any
> possible bug which can be hit by old test.
> 
> CC: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> CC: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Oops, thought I applied them.

Applied to wq/for-3.17 w/ minor updates.

Thanks.

-- 
tejun