On Tue, May 12, 2015 at 08:32:32PM +0800, Lai Jiangshan wrote:
> If the cpuamsk is changed, it is possible that only a part of the per-node

         cpumask

> pwq is affected.  This can happen when the user changes the cpumask of
> a workqueue or the low level cpumask.
> 
> So we try to reuse the current per-node pwq when its attrs unchanged.

                                                             are unchanged.

> @@ -3592,9 +3593,14 @@ apply_wqattrs_prepare(struct workqueue_struct *wq,
>  
>       for_each_node(node) {
>               if (wq_calc_node_cpumask(new_attrs, node, -1, 
> tmp_attrs->cpumask)) {
> -                     ctx->pwq_tbl[node] = alloc_unbound_pwq(wq, tmp_attrs);
> -                     if (!ctx->pwq_tbl[node])
> +                     pwq = unbound_pwq_by_node(wq, node);
> +                     if (pwq && wqattrs_equal(tmp_attrs, pwq->pool->attrs))
> +                             pwq = get_pwq_unlocked(pwq);

Ah, okay, the function gets used here again.  BTW, why does this
function return anything?  Can this function ever return something
which isn't the pwq it was called with?

> +                     else
> +                             pwq = alloc_unbound_pwq(wq, tmp_attrs);
> +                     if (!pwq)
>                               goto out_free;

If get_pwq_unlocked() can't fail, why are we testing for NULL pwq
here?  This code is kinda misleading.

> +                     ctx->pwq_tbl[node] = pwq;
>               } else {
>                       ctx->dfl_pwq->refcnt++;
>                       ctx->pwq_tbl[node] = ctx->dfl_pwq;
> @@ -3739,7 +3745,6 @@ static void wq_update_unbound_numa(struct 
> workqueue_struct *wq, int cpu,
>       cpumask = target_attrs->cpumask;
>  
>       copy_workqueue_attrs(target_attrs, wq->unbound_attrs);
> -     pwq = unbound_pwq_by_node(wq, node);
>  
>       /*
>        * Let's determine what needs to be done.  If the target cpumask is
> @@ -3748,6 +3753,7 @@ static void wq_update_unbound_numa(struct 
> workqueue_struct *wq, int cpu,
>        * equals the default pwq's, the default pwq should be used.
>        */
>       if (wq_calc_node_cpumask(wq->dfl_pwq->pool->attrs, node, cpu_off, 
> cpumask)) {
> +             pwq = unbound_pwq_by_node(wq, node);

It'd be nice to note this change in the patch description.

Thanks.

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

Reply via email to