From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Jiri Slaby <jslaby@suse.cz>
Cc: linux-kernel@vger.kernel.org, jirislaby@gmail.com,
Vojtech Pavlik <vojtech@suse.cz>, Michael Matz <matz@suse.de>,
Jiri Kosina <jkosina@suse.cz>,
Steven Rostedt <rostedt@goodmis.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@redhat.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Theodore Ts'o" <tytso@mit.edu>,
Dipankar Sarma <dipankar@in.ibm.com>, Tejun Heo <tj@kernel.org>
Subject: Re: [RFC 09/16] kgr: mark task_safe in some kthreads
Date: Wed, 30 Apr 2014 09:55:32 -0700 [thread overview]
Message-ID: <20140430165532.GS8754@linux.vnet.ibm.com> (raw)
In-Reply-To: <1398868249-26169-10-git-send-email-jslaby@suse.cz>
On Wed, Apr 30, 2014 at 04:30:42PM +0200, Jiri Slaby wrote:
> Some threads do not use kthread_should_stop. Before we enable a
> kthread support in kgr, we must make sure all those mark themselves
> safe explicitly.
Would it make sense to bury kgr_task_safe() in wait_event_interruptible()
and friends? The kgr_task_safe() implementation looks pretty lightweight,
so it should not be a performance problem.
One reason might this might be a bad idea is that there are calls to
wait_event_interruptible() all over the place, which might therefore
constrain where grafting could be safely done. That would be fair enough,
but does that also imply new constraints on where kthread_should_stop()
can be invoked? Any new constraints might not be a big deal given that
a very large fraction of the kthreads (and maybe all of them) invoke
kthread_should_stop() from their top-level function, but would be good
to call out.
So, what is the story?
Thanx, Paul
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Dipankar Sarma <dipankar@in.ibm.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Tejun Heo <tj@kernel.org>
> ---
> drivers/base/devtmpfs.c | 1 +
> fs/jbd2/journal.c | 2 ++
> fs/notify/mark.c | 5 ++++-
> kernel/hung_task.c | 5 ++++-
> kernel/kthread.c | 3 +++
> kernel/rcu/tree.c | 6 ++++--
> kernel/rcu/tree_plugin.h | 9 +++++++--
> kernel/workqueue.c | 1 +
> 8 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index 25798db14553..c7d52d1b8c9c 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -387,6 +387,7 @@ static int devtmpfsd(void *p)
> sys_chroot(".");
> complete(&setup_done);
> while (1) {
> + kgr_task_safe(current);
> spin_lock(&req_lock);
> while (requests) {
> struct req *req = requests;
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 67b8e303946c..1b9c4c2e014a 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -43,6 +43,7 @@
> #include <linux/backing-dev.h>
> #include <linux/bitops.h>
> #include <linux/ratelimit.h>
> +#include <linux/sched.h>
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/jbd2.h>
> @@ -260,6 +261,7 @@ loop:
> write_lock(&journal->j_state_lock);
> }
> finish_wait(&journal->j_wait_commit, &wait);
> + kgr_task_safe(current);
> }
>
> jbd_debug(1, "kjournald2 wakes\n");
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 923fe4a5f503..a74b6175e645 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -82,6 +82,7 @@
> #include <linux/kthread.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/sched.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/srcu.h>
> @@ -355,7 +356,9 @@ static int fsnotify_mark_destroy(void *ignored)
> fsnotify_put_mark(mark);
> }
>
> - wait_event_interruptible(destroy_waitq, !list_empty(&destroy_list));
> + wait_event_interruptible(destroy_waitq, ({
> + kgr_task_safe(current);
> + !list_empty(&destroy_list); }));
> }
>
> return 0;
> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index 06bb1417b063..b5f85bff2509 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -14,6 +14,7 @@
> #include <linux/kthread.h>
> #include <linux/lockdep.h>
> #include <linux/export.h>
> +#include <linux/sched.h>
> #include <linux/sysctl.h>
> #include <linux/utsname.h>
> #include <trace/events/sched.h>
> @@ -227,8 +228,10 @@ static int watchdog(void *dummy)
> for ( ; ; ) {
> unsigned long timeout = sysctl_hung_task_timeout_secs;
>
> - while (schedule_timeout_interruptible(timeout_jiffies(timeout)))
> + while (schedule_timeout_interruptible(timeout_jiffies(timeout))) {
> + kgr_task_safe(current);
> timeout = sysctl_hung_task_timeout_secs;
> + }
>
> if (atomic_xchg(&reset_hung_task, 0))
> continue;
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 9a130ec06f7a..08b979dad619 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -78,6 +78,8 @@ static struct kthread *to_live_kthread(struct task_struct *k)
> */
> bool kthread_should_stop(void)
> {
> + kgr_task_safe(current);
> +
> return test_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags);
> }
> EXPORT_SYMBOL(kthread_should_stop);
> @@ -497,6 +499,7 @@ int kthreadd(void *unused)
> if (list_empty(&kthread_create_list))
> schedule();
> __set_current_state(TASK_RUNNING);
> + kgr_task_safe(current);
>
> spin_lock(&kthread_create_lock);
> while (!list_empty(&kthread_create_list)) {
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 0c47e300210a..5dddedacfc06 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1593,9 +1593,10 @@ static int __noreturn rcu_gp_kthread(void *arg)
> trace_rcu_grace_period(rsp->name,
> ACCESS_ONCE(rsp->gpnum),
> TPS("reqwait"));
> - wait_event_interruptible(rsp->gp_wq,
> + wait_event_interruptible(rsp->gp_wq, ({
> + kgr_task_safe(current);
> ACCESS_ONCE(rsp->gp_flags) &
> - RCU_GP_FLAG_INIT);
> + RCU_GP_FLAG_INIT; }));
> /* Locking provides needed memory barrier. */
> if (rcu_gp_init(rsp))
> break;
> @@ -1626,6 +1627,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
> (!ACCESS_ONCE(rnp->qsmask) &&
> !rcu_preempt_blocked_readers_cgp(rnp)),
> j);
> + kgr_task_safe(current);
> /* Locking provides needed memory barriers. */
> /* If grace period done, leave loop. */
> if (!ACCESS_ONCE(rnp->qsmask) &&
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 962d1d589929..8b383003b228 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -27,6 +27,7 @@
> #include <linux/delay.h>
> #include <linux/gfp.h>
> #include <linux/oom.h>
> +#include <linux/sched.h>
> #include <linux/smpboot.h>
> #include "../time/tick-internal.h"
>
> @@ -1273,7 +1274,8 @@ static int rcu_boost_kthread(void *arg)
> for (;;) {
> rnp->boost_kthread_status = RCU_KTHREAD_WAITING;
> trace_rcu_utilization(TPS("End boost kthread@rcu_wait"));
> - rcu_wait(rnp->boost_tasks || rnp->exp_tasks);
> + rcu_wait(({ kgr_task_safe(current);
> + rnp->boost_tasks || rnp->exp_tasks; }));
> trace_rcu_utilization(TPS("Start boost kthread@rcu_wait"));
> rnp->boost_kthread_status = RCU_KTHREAD_RUNNING;
> more2boost = rcu_boost(rnp);
> @@ -2283,11 +2285,14 @@ static int rcu_nocb_kthread(void *arg)
>
> /* Each pass through this loop invokes one batch of callbacks */
> for (;;) {
> + kgr_task_safe(current);
> /* If not polling, wait for next batch of callbacks. */
> if (!rcu_nocb_poll) {
> trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
> TPS("Sleep"));
> - wait_event_interruptible(rdp->nocb_wq, rdp->nocb_head);
> + wait_event_interruptible(rdp->nocb_wq, ({
> + kgr_task_safe(current);
> + rdp->nocb_head; }));
> /* Memory barrier provide by xchg() below. */
> } else if (firsttime) {
> firsttime = 0;
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 0ee63af30bd1..4b89f1dc0dd8 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2369,6 +2369,7 @@ sleep:
> __set_current_state(TASK_INTERRUPTIBLE);
> spin_unlock_irq(&pool->lock);
> schedule();
> + kgr_task_safe(current);
> goto woke_up;
> }
>
> --
> 1.9.2
>
next prev parent reply other threads:[~2014-04-30 16:55 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-30 14:30 [RFC 00/16] kGraft Jiri Slaby
2014-04-30 14:30 ` [RFC 01/16] ftrace: Add function to find fentry of function Jiri Slaby
2014-04-30 14:48 ` Steven Rostedt
2014-04-30 14:58 ` Jiri Slaby
2014-04-30 14:30 ` [RFC 02/16] ftrace: Make ftrace_is_dead available globally Jiri Slaby
2014-04-30 14:30 ` [RFC 03/16] kgr: initial code Jiri Slaby
2014-04-30 14:56 ` Steven Rostedt
2014-04-30 14:57 ` Jiri Slaby
2014-05-01 20:20 ` Andi Kleen
2014-05-01 20:37 ` Jiri Kosina
2014-05-14 9:28 ` Aravinda Prasad
2014-05-14 10:12 ` Jiri Slaby
2014-05-14 10:41 ` Aravinda Prasad
2014-05-14 10:44 ` Jiri Slaby
2014-05-14 11:19 ` Aravinda Prasad
2014-05-20 11:36 ` Jiri Slaby
2014-05-21 18:28 ` Aravinda Prasad
2014-05-26 8:50 ` Jiri Kosina
2014-04-30 14:30 ` [RFC 04/16] kgr: add testing kgraft patch Jiri Slaby
2014-05-06 11:03 ` Pavel Machek
2014-05-12 12:50 ` Jiri Slaby
2014-04-30 14:30 ` [RFC 05/16] kgr: update Kconfig documentation Jiri Slaby
2014-05-03 14:32 ` Randy Dunlap
2014-04-30 14:30 ` [RFC 06/16] kgr: add Documentation Jiri Slaby
2014-05-06 11:03 ` Pavel Machek
2014-05-09 9:31 ` kgr: dealing with optimalizations? (was Re: [RFC 06/16] kgr: add Documentat)ion Pavel Machek
2014-05-09 12:22 ` Michael Matz
2014-04-30 14:30 ` [RFC 07/16] kgr: trigger the first check earlier Jiri Slaby
2014-04-30 14:30 ` [RFC 08/16] kgr: sched.h, introduce kgr_task_safe helper Jiri Slaby
2014-04-30 14:30 ` [RFC 09/16] kgr: mark task_safe in some kthreads Jiri Slaby
2014-04-30 15:49 ` Greg Kroah-Hartman
2014-04-30 16:55 ` Paul E. McKenney [this message]
2014-04-30 18:33 ` Vojtech Pavlik
2014-04-30 19:07 ` Paul E. McKenney
2014-05-01 14:24 ` Tejun Heo
2014-05-01 20:17 ` Jiri Kosina
2014-05-01 21:02 ` Tejun Heo
2014-05-01 21:09 ` Tejun Heo
2014-05-14 14:59 ` Jiri Slaby
2014-05-14 15:15 ` Vojtech Pavlik
2014-05-14 15:30 ` Paul E. McKenney
2014-05-14 16:32 ` Tejun Heo
2014-05-15 3:53 ` Mike Galbraith
2014-05-15 4:06 ` Tejun Heo
2014-05-15 4:46 ` Mike Galbraith
2014-05-15 4:50 ` Tejun Heo
2014-05-15 5:04 ` Mike Galbraith
2014-05-15 5:09 ` Tejun Heo
2014-05-15 5:32 ` Mike Galbraith
2014-05-15 6:05 ` Tejun Heo
2014-05-15 6:32 ` Mike Galbraith
2014-04-30 14:30 ` [RFC 10/16] kgr: kthreads support Jiri Slaby
2014-04-30 14:30 ` [RFC 11/16] kgr: handle irqs Jiri Slaby
2014-04-30 14:30 ` [RFC 12/16] kgr: add tools Jiri Slaby
2014-05-06 11:03 ` Pavel Machek
2014-04-30 14:30 ` [RFC 13/16] kgr: add MAINTAINERS entry Jiri Slaby
2014-04-30 14:30 ` [RFC 14/16] kgr: x86: refuse to build without fentry support Jiri Slaby
2014-04-30 14:30 ` [RFC 15/16] kgr: add procfs interface for per-process 'kgr_in_progress' Jiri Slaby
2014-04-30 14:30 ` [RFC 16/16] kgr: make a per-process 'in progress' flag a single bit Jiri Slaby
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=20140430165532.GS8754@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=dipankar@in.ibm.com \
--cc=fweisbec@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@gmail.com \
--cc=jkosina@suse.cz \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=matz@suse.de \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=tj@kernel.org \
--cc=tytso@mit.edu \
--cc=vojtech@suse.cz \
/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;
as well as URLs for NNTP newsgroup(s).