linux-kernel - Re: [PATCH 6/6] workqueue: Record real per-workqueue cpumask

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]

Date:	Thu, 8 May 2014 21:20:45 +0800
From:	Lai Jiangshan <laijs@...fujitsu.com>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Christoph Lameter <cl@...ux.com>,
	Kevin Hilman <khilman@...aro.org>,
	Mike Galbraith <bitbucket@...ine.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Tejun Heo <tj@...nel.org>,
	Viresh Kumar <viresh.kumar@...aro.org>
Subject: Re: [PATCH 6/6] workqueue: Record real per-workqueue cpumask

On Thu, May 8, 2014 at 12:37 AM, Frederic Weisbecker <fweisbec@...il.com> wrote:
> The real cpumask set by the user on WQ_SYSFS workqueues fails to be
> recorded as is: What is actually recorded as per workqueue attribute
> is the per workqueue cpumask intersected with the global unbounds cpumask.
>
> Eventually when the user overwrites a WQ_SYSFS cpumask and later read
> this attibute, the value returned is not the last one written.
>
> The other bad side effect is that widening the global unbounds cpumask
> doesn't actually widen the unbound workqueues affinity because their
> own cpumask has been schrinked.
>
> In order to fix this, lets record the real per workqueue cpumask on the
> workqueue struct. We restore this value when attributes are re-evaluated
> later.
>
> FIXME: Maybe I should rather invert that. Have the user set workqueue
> cpumask on attributes and the effective one on the workqueue struct instead.
> We'll just need some tweaking in order to make the attributes of lower layers
> (pools, worker pools, worker, ...) to inherit the effective cpumask and not
> the user one.
>
> Cc: Christoph Lameter <cl@...ux.com>
> Cc: Kevin Hilman <khilman@...aro.org>
> Cc: Lai Jiangshan <laijs@...fujitsu.com>
> Cc: Mike Galbraith <bitbucket@...ine.de>
> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Cc: Tejun Heo <tj@...nel.org>
> Cc: Viresh Kumar <viresh.kumar@...aro.org>
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> ---
>  kernel/workqueue.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 5978cee..504cf0a 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -248,6 +248,7 @@ struct workqueue_struct {
>         int                     saved_max_active; /* WQ: saved pwq max_active */
>
>         struct workqueue_attrs  *unbound_attrs; /* WQ: only for unbound wqs */
> +       cpumask_var_t           saved_cpumask;  /* WQ: only for unbound wqs */


Forgot to use it? or use it in next patches?

>         struct pool_workqueue   *dfl_pwq;       /* WQ: only for unbound wqs */
>
>  #ifdef CONFIG_SYSFS
> @@ -3694,6 +3695,7 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq,
>         mutex_lock(&wq->mutex);
>
>         copy_workqueue_attrs(wq->unbound_attrs, new_attrs);
> +       cpumask_copy(wq->saved_cpumask, attrs->cpumask);

I think you can use ->unbound_attrs directly:
          copy_workqueue_attrs(wq->unbound_attrs, attrs);

and update wq_update_unbound_numa():
         copy_workqueue_attrs(tmp_attrs, wq->unbound_attrs);
         cpumask_and(&tmp_attrs->cpumask, wq_unbound_cpumask)

use tmp_attr instead of wq->unbound_attrs in the left code of
wq_update_unbound_numa()

>
>         /* save the previous pwq and install the new one */
>         for_each_node(node)
> @@ -4326,6 +4328,11 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
>                 wq->unbound_attrs = alloc_workqueue_attrs(GFP_KERNEL);
>                 if (!wq->unbound_attrs)
>                         goto err_free_wq;
> +
> +               if (!alloc_cpumask_var(&wq->saved_cpumask, GFP_KERNEL))
> +                       goto err_free_wq;
> +
> +               cpumask_copy(wq->saved_cpumask, cpu_possible_mask);
>         }
>
>         va_start(args, lock_name);
> @@ -4397,6 +4404,7 @@ struct workqueue_struct *__alloc_workqueue_key(const char *fmt,
>         return wq;
>
>  err_free_wq:
> +       free_cpumask_var(wq->saved_cpumask);
>         free_workqueue_attrs(wq->unbound_attrs);
>         kfree(wq);
>         return NULL;
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/