On Thu, May 08, 2014 at 09:20:45PM +0800, Lai Jiangshan wrote:
> On Thu, May 8, 2014 at 12:37 AM, Frederic Weisbecker <fweis...@gmail.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 <c...@linux.com>
> > Cc: Kevin Hilman <khil...@linaro.org>
> > Cc: Lai Jiangshan <la...@cn.fujitsu.com>
> > Cc: Mike Galbraith <bitbuc...@online.de>
> > Cc: Paul E. McKenney <paul...@linux.vnet.ibm.com>
> > Cc: Tejun Heo <t...@kernel.org>
> > Cc: Viresh Kumar <viresh.ku...@linaro.org>
> > Signed-off-by: Frederic Weisbecker <fweis...@gmail.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?
Hmm, no it's used below.
>
> > 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()
But wq_update_unbound_numa() is only called on cpu hotplug operations
right? So this may have no effect after setting a cpumask in sysfs.
How about keeping the sysfs cpu in wq's unbound_attrs but pass the effective
one to pwq creation in apply_workqueue_attrs_locked.
And also do what you suggest in wq_update_unbound_numa for hotplug
operations.
--
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/
