From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@kernel.org, riel@redhat.com, dedekind1@gmail.com,
linux-kernel@vger.kernel.org, mgorman@suse.de,
rostedt@goodmis.org, juri.lelli@arm.com,
Oleg Nesterov <oleg@redhat.com>
Subject: Re: [RFC][PATCH 1/4] sched: Fix a race between __kthread_bind() and sched_setaffinity()
Date: Fri, 7 Aug 2015 11:16:08 -0400 [thread overview]
Message-ID: <20150807151608.GD14626@mtj.duckdns.org> (raw)
In-Reply-To: <20150807142708.GK16853@twins.programming.kicks-ass.net>
On Fri, Aug 07, 2015 at 04:27:08PM +0200, Peter Zijlstra wrote:
> Which is the rescue thread attaching itself to a pool that needs help,
> and obviously the rescue thread isn't new so kthread_bind doesn't work
> right.
>
> The best I could come up with is something like the below on top; does
> that work for you? I'll go give it some runtime.
>
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1622,11 +1622,15 @@ static struct worker *alloc_worker(int n
> * cpu-[un]hotplugs.
> */
> static void worker_attach_to_pool(struct worker *worker,
> - struct worker_pool *pool)
> + struct worker_pool *pool,
> + bool new)
> {
> mutex_lock(&pool->attach_mutex);
>
> - kthread_bind_mask(worker->task, pool->attrs->cpumask);
> + if (new)
> + kthread_bind_mask(worker->task, pool->attrs->cpumask);
> + else
> + set_cpus_allowed_ptr(worker->task, pool->attrs->cpumask);
>
> /*
> * The pool->attach_mutex ensures %POOL_DISASSOCIATED remains
> @@ -1712,7 +1716,7 @@ static struct worker *create_worker(stru
> set_user_nice(worker->task, pool->attrs->nice);
>
> /* successful, attach the worker to the pool */
> - worker_attach_to_pool(worker, pool);
> + worker_attach_to_pool(worker, pool, true);
>
> /* start the newly created worker */
> spin_lock_irq(&pool->lock);
> @@ -2241,7 +2245,7 @@ static int rescuer_thread(void *__rescue
>
> spin_unlock_irq(&wq_mayday_lock);
>
> - worker_attach_to_pool(rescuer, pool);
> + worker_attach_to_pool(rescuer, pool, false);
Hmmm... the race condition didn't exist for workqueue in the first
place, right? As long as the flag is set before the affinity is
configured, there's no race condition. I think the code was better
before. Can't we just revert workqueue.c part?
Thanks.
--
tejun
next prev parent reply other threads:[~2015-08-07 15:16 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-15 15:43 [RFC][PATCH 0/4] sched,numa: pinned task accounting Peter Zijlstra
2015-05-15 15:43 ` [RFC][PATCH 1/4] sched: Fix a race between __kthread_bind() and sched_setaffinity() Peter Zijlstra
2015-05-15 15:56 ` Tejun Heo
2015-08-07 14:27 ` Peter Zijlstra
2015-08-07 15:16 ` Tejun Heo [this message]
2015-08-07 15:29 ` Peter Zijlstra
2015-08-07 15:38 ` Tejun Heo
2015-08-07 15:59 ` Peter Zijlstra
2015-08-07 16:09 ` Tejun Heo
2015-08-12 12:38 ` [tip:sched/core] " tip-bot for Peter Zijlstra
2015-05-15 15:43 ` [RFC][PATCH 2/4] sched: Make sched_class::set_cpus_allowed() unconditional Peter Zijlstra
2015-08-12 12:38 ` [tip:sched/core] " tip-bot for Peter Zijlstra
2015-08-20 16:45 ` [RFC][PATCH 2/4] " Sasha Levin
2015-05-15 15:43 ` [RFC][PATCH 3/4] sched: Change sched_class::set_cpus_allowed calling context Peter Zijlstra
[not found] ` <OF66BF3765.2EBFD3B1-ON48257E49.0028DC79-48257E49.0029F058@zte.com.cn>
2015-05-18 8:32 ` Peter Zijlstra
2015-05-18 9:34 ` Juri Lelli
2015-05-18 20:04 ` Peter Zijlstra
2015-08-12 12:38 ` [tip:sched/core] sched: Change the sched_class::set_cpus_allowed( ) " tip-bot for Peter Zijlstra
2015-08-13 18:47 ` Sasha Levin
2015-08-13 20:37 ` Peter Zijlstra
2015-08-13 20:59 ` Sasha Levin
2015-08-13 21:30 ` Peter Zijlstra
2015-05-15 15:43 ` [RFC][PATCH 4/4] sched, numa: Ignore pinned tasks Peter Zijlstra
2015-05-15 19:05 ` Rik van Riel
2015-05-16 9:31 ` Peter Zijlstra
2015-05-18 13:00 ` Srikar Dronamraju
2015-05-18 13:06 ` Peter Zijlstra
2015-05-18 14:13 ` Rik van Riel
2015-05-18 14:29 ` Srikar Dronamraju
2015-05-18 15:09 ` Peter Zijlstra
2015-05-18 13:10 ` Srikar Dronamraju
2015-05-18 9:08 ` [RFC][PATCH 0/4] sched,numa: pinned task accounting Artem Bityutskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150807151608.GD14626@mtj.duckdns.org \
--to=tj@kernel.org \
--cc=dedekind1@gmail.com \
--cc=juri.lelli@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=riel@redhat.com \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome