[PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Lai Jiangshan]

There is a piece of sanity checks code in the put_unbound_pool().
The meaning of this code is "if it is not an unbound pool, it will complain
and return" IIUC. But the code uses "pool->flags & POOL_DISASSOCIATED"
imprecisely due to a non-unbound pool may also have this flags.

We should use "pool->cpu < 0" to stand for an unbound pool, so we covert the
code to it.

There is no strictly wrong if we still keep "pool->flags & POOL_DISASSOCIATED"
here, but it is just a noise if we keep it:
  1) we focus on "unbound" here, not "[dis]association".
  2) "pool->cpu < 0" already implies "pool->flags & POOL_DISASSOCIATED".

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

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 90a0fa5..724ae35 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3457,7 +3457,7 @@ static void put_unbound_pool(struct worker_pool *pool)
 		return;
 
 	/* sanity checks */
-	if (WARN_ON(!(pool->flags & POOL_DISASSOCIATED)) ||
+	if (WARN_ON(!(pool->cpu < 0)) ||
 	    WARN_ON(!list_empty(&pool->worklist)))
 		return;
 
-- 
1.7.4.4

Re: [PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Lai Jiangshan]

Ping.

And would these patches be possible for 3.16?

Thanks,
Lai

On 06/03/2014 03:31 PM, Lai Jiangshan wrote:
> There is a piece of sanity checks code in the put_unbound_pool().
> The meaning of this code is "if it is not an unbound pool, it will complain
> and return" IIUC. But the code uses "pool->flags & POOL_DISASSOCIATED"
> imprecisely due to a non-unbound pool may also have this flags.
> 
> We should use "pool->cpu < 0" to stand for an unbound pool, so we covert the
> code to it.
> 
> There is no strictly wrong if we still keep "pool->flags & POOL_DISASSOCIATED"
> here, but it is just a noise if we keep it:
>   1) we focus on "unbound" here, not "[dis]association".
>   2) "pool->cpu < 0" already implies "pool->flags & POOL_DISASSOCIATED".
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
>  kernel/workqueue.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 90a0fa5..724ae35 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -3457,7 +3457,7 @@ static void put_unbound_pool(struct worker_pool *pool)
>  		return;
>  
>  	/* sanity checks */
> -	if (WARN_ON(!(pool->flags & POOL_DISASSOCIATED)) ||
> +	if (WARN_ON(!(pool->cpu < 0)) ||
>  	    WARN_ON(!list_empty(&pool->worklist)))
>  		return;
>  

Re: [PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Tejun Heo]

On Thu, Jun 05, 2014 at 09:20:56AM +0800, Lai Jiangshan wrote:
> And would these patches be possible for 3.16?

It's a bit too late.  I'd like to wait for the next merge window.
AFAICS, there's nothing critical, right?

Thanks.

-- 
tejun

Re: [PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Lai Jiangshan]

On 06/05/2014 09:17 AM, Tejun Heo wrote:
> On Thu, Jun 05, 2014 at 09:20:56AM +0800, Lai Jiangshan wrote:
>> And would these patches be possible for 3.16?
> 
> It's a bit too late.  I'd like to wait for the next merge window.

OK,

> AFAICS, there's nothing critical, right?

They are normal cleanups.

Thanks,

Re: [PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Lai Jiangshan]

Hi, Tejun

3.16-rc1 came out. Could you review the patches?

Thanks,
Lai

On 06/03/2014 03:31 PM, Lai Jiangshan wrote:
> There is a piece of sanity checks code in the put_unbound_pool().
> The meaning of this code is "if it is not an unbound pool, it will complain
> and return" IIUC. But the code uses "pool->flags & POOL_DISASSOCIATED"
> imprecisely due to a non-unbound pool may also have this flags.
> 
> We should use "pool->cpu < 0" to stand for an unbound pool, so we covert the
> code to it.
> 
> There is no strictly wrong if we still keep "pool->flags & POOL_DISASSOCIATED"
> here, but it is just a noise if we keep it:
>   1) we focus on "unbound" here, not "[dis]association".
>   2) "pool->cpu < 0" already implies "pool->flags & POOL_DISASSOCIATED".
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
>  kernel/workqueue.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 90a0fa5..724ae35 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -3457,7 +3457,7 @@ static void put_unbound_pool(struct worker_pool *pool)
>  		return;
>  
>  	/* sanity checks */
> -	if (WARN_ON(!(pool->flags & POOL_DISASSOCIATED)) ||
> +	if (WARN_ON(!(pool->cpu < 0)) ||
>  	    WARN_ON(!list_empty(&pool->worklist)))
>  		return;
>  

Re: [PATCH] workqueue: use "pool->cpu < 0" to stand for an unbound pool

[Tejun Heo]

On Tue, Jun 03, 2014 at 03:31:45PM +0800, Lai Jiangshan wrote:
> There is a piece of sanity checks code in the put_unbound_pool().
> The meaning of this code is "if it is not an unbound pool, it will complain
> and return" IIUC. But the code uses "pool->flags & POOL_DISASSOCIATED"
> imprecisely due to a non-unbound pool may also have this flags.
> 
> We should use "pool->cpu < 0" to stand for an unbound pool, so we covert the
> code to it.
> 
> There is no strictly wrong if we still keep "pool->flags & POOL_DISASSOCIATED"
> here, but it is just a noise if we keep it:
>   1) we focus on "unbound" here, not "[dis]association".
>   2) "pool->cpu < 0" already implies "pool->flags & POOL_DISASSOCIATED".
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Applied to wq/for-3.17.

Thansk.

-- 
tejun