On 04/28/2015 11:49 AM, Tejun Heo wrote:
> Hello,
>
> On Tue, Apr 28, 2015 at 10:24:31AM +0800, Lai Jiangshan wrote:
>>>> Wouldn't this make a lot more sense above when copying @attrs into
>>>> @new_attrs? The comment there even says "make a copy of @attrs and
>>>> sanitize it". Copy to @new_attrs, mask with wq_unbound_cpumask and
>>>> fall back to wq_unbound_cpumask if empty.
>>
>> We need to save the user original configured attrs.
>> When any time wq_unbound_cpumask is changed, we should use
>> the user original configured attrs (cpumask) to re-calculate
>> the pwqs and avoid losing any information.
>
> Sure, we can do that for new_attrs and then mask tmp_attrs further w/
> wq_unbound_cpumask, no?
>
> Thanks.
>
We need to pass new_attrs to wq_calc_node_cpumask().
If new_attrs (the first argument of wq_calc_node_cpumask()) is not masked
with wq_unbound_cpumask when passed in, wq_calc_node_cpumask()
will be much complicated (I tried coding it yesterday).
Quote:
static bool wq_calc_node_cpumask(const struct workqueue_attrs *attrs, int node,
int cpu_going_down, cpumask_t *cpumask)
{
if (!wq_numa_enabled || attrs->no_numa)
goto use_dfl;
/* does @node have any online CPUs @attrs wants? */
cpumask_and(cpumask, cpumask_of_node(node), attrs->cpumask); [1]
if (cpu_going_down >= 0)
cpumask_clear_cpu(cpu_going_down, cpumask);
if (cpumask_empty(cpumask))
goto use_dfl;
/* yeap, return possible CPUs in @node that @attrs wants */
cpumask_and(cpumask, attrs->cpumask, wq_numa_possible_cpumask[node]);
[2]
return !cpumask_equal(cpumask, attrs->cpumask); [3]
use_dfl:
cpumask_copy(cpumask, attrs->cpumask); [4]
return false;
}
If @attrs is not masked with wq_unbound_cpumask when passed in, the code
needs add two maskings (with wq_unbound_cpumask) at [1] and [2].
And the code requests to get the cpumask of the default pwq at [3]&[4],
thus the code need to (re-)calculate the default pwq's attrs here and
doubles the code. (this calculation is already done before this function).
It will make all things simple and avoid complicating the
wq_calc_node_cpumask(),
if wq_calc_node_cpumask() is kept unchanged but accepts only the default pwq's
attrs as its first argument.
The call-site in wq_update_unbound_numa() is changed in V8 to meet this
requirement.
@@ -3705,11 +3714,11 @@ static void wq_update_unbound_numa(struct
workqueue_struct *wq, int cpu,
/*
* Let's determine what needs to be done. If the target cpumask is
- * different from wq's, we need to compare it to @pwq's and create
- * a new one if they don't match. If the target cpumask equals
- * wq's, the default pwq should be used.
+ * different from the default pwq's, we need to compare it to @pwq's
+ * and create a new one if they don't match. If the target cpumask
+ * equals the default pwq's, the default pwq should be used.
*/
- if (wq_calc_node_cpumask(wq->unbound_attrs, node, cpu_off, cpumask)) {
+ if (wq_calc_node_cpumask(wq->dfl_pwq->pool->attrs, node, cpu_off,
cpumask)) {
if (cpumask_equal(cpumask, pwq->pool->attrs->cpumask))
goto out_unlock;
} else {
This requirement is not a new requirement. In the code before this patch,
the argument @attrs for wq_calc_node_cpumask() is expected to be the default
pwq's attrs which happens to be wq->unbound_attrs all the time.
In the code after this patch, the argument @attrs for wq_calc_node_cpumask()
is still expected to be the default pwq's attrs which may not be
wq->unbound_attrs.
So the requirement is not new and wq_calc_node_cpumask() is untouched,
but the comment for wq_calc_node_cpumask() needs to be updated which
I should have done, forgive me.
Thanks,
Lai.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
