All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: paulmck@linux.vnet.ibm.com, josh@joshtriplett.org,
	rostedt@goodmis.org, mathieu.desnoyers@efficios.com,
	jiangshanlai@gmail.com
Cc: paul.gortmaker@windriver.com, ebiederm@xmission.com,
	dmitry.torokhov@gmail.com, linux-kernel@vger.kernel.org,
	"Luis R. Rodriguez" <mcgrof@kernel.org>
Subject: [RFC] rcu: use killable versions of swait
Date: Wed, 14 Jun 2017 16:06:39 -0700	[thread overview]
Message-ID: <20170614230639.15079-1-mcgrof@kernel.org> (raw)

These waits don't even check for the return value for interruption,
using the non-killable variants means we could be killed by other
signals than SIGKILL, this is fragile.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---

The killable swaits were just posted [1] as part of a series where SIGCHLD
was detected as interrupting and killing kernel calls waiting using
non-killable swaits [1]. The fragility here made curious about other callers
and seeing if they really meant to use such broad wait which captures a lot
of signals.

I can't see why we'd want to have these killed by other signals, specialy
since it seems we don't even check for the return value... Granted to abort
properly we'd have to check for the return value for -ERESTARTSYS, but yeah,
none of this is done, so it would seem we don't want fragile signals
interrupting these ?

Also can someone confirm if the original change of to swait_event_timeout()
from wait_event_interruptible_timeout() was actually intentional on
synchronize_sched_expedited_wait() on commit abedf8e2419fb ("rcu: Use simple
wait queues where possible in rcutree") ? I can't easily confirm.

[0] https://lkml.kernel.org/r/20170614222017.14653-3-mcgrof@kernel.org
[1] https://lkml.kernel.org/r/20170614222017.14653-1-mcgrof@kernel.org

 kernel/rcu/tree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 695fee7cafe0..9a8d06486a3c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2191,7 +2191,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					       READ_ONCE(rsp->gpnum),
 					       TPS("reqwait"));
 			rsp->gp_state = RCU_GP_WAIT_GPS;
-			swait_event_interruptible(rsp->gp_wq,
+			swait_event_killable(rsp->gp_wq,
 						 READ_ONCE(rsp->gp_flags) &
 						 RCU_GP_FLAG_INIT);
 			rsp->gp_state = RCU_GP_DONE_GPS;
@@ -2224,7 +2224,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
 					       READ_ONCE(rsp->gpnum),
 					       TPS("fqswait"));
 			rsp->gp_state = RCU_GP_WAIT_FQS;
-			ret = swait_event_interruptible_timeout(rsp->gp_wq,
+			ret = swait_event_killable_timeout(rsp->gp_wq,
 					rcu_gp_fqs_check_wake(rsp, &gf), j);
 			rsp->gp_state = RCU_GP_DOING_FQS;
 			/* Locking provides needed memory barriers. */
-- 
2.11.0

             reply	other threads:[~2017-06-14 23:06 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-14 23:06 Luis R. Rodriguez [this message]
2017-06-14 23:43 ` [RFC] rcu: use killable versions of swait Paul E. McKenney
2017-06-15 15:50   ` Luis R. Rodriguez
2017-06-15 16:22     ` Paul E. McKenney
2017-06-15 16:35       ` Luis R. Rodriguez
2017-06-15 16:55         ` Eric W. Biederman
2017-06-15 18:48           ` [RFC v2 0/2] swait: add idle to make idle-hacks on kthreads explicit Luis R. Rodriguez
2017-06-15 18:48             ` [RFC v2 1/2] swait: add idle variants which don't contribute to load average Luis R. Rodriguez
2017-06-16  0:47               ` Boqun Feng
2017-06-20 21:32                 ` Luis R. Rodriguez
2017-06-16 20:31               ` Eric W. Biederman
2017-06-19 17:53                 ` Paul E. McKenney
2017-06-15 18:48             ` [RFC v2 2/2] rcu: use idle versions of swait to make idle-hack clear Luis R. Rodriguez
2017-06-15 21:57             ` [RFC v2 0/2] swait: add idle to make idle-hacks on kthreads explicit Paul E. McKenney
2017-06-15 23:26               ` Luis R. Rodriguez
2017-06-15 23:43                 ` Paul E. McKenney
2017-06-16 20:37                   ` Eric W. Biederman
2017-06-19 17:54                     ` Paul E. McKenney
2017-06-20 21:45             ` [PATCH " Luis R. Rodriguez
2017-06-20 21:45               ` [PATCH 1/2] swait: add idle variants which don't contribute to load average Luis R. Rodriguez
2017-06-20 21:45               ` [PATCH 2/2] rcu: use idle versions of swait to make idle-hack clear Luis R. Rodriguez
2017-06-21 16:48               ` [PATCH 0/2] swait: add idle to make idle-hacks on kthreads explicit Paul E. McKenney
2017-06-21 17:57                 ` Luis R. Rodriguez
2017-06-21 18:19                   ` Paul E. McKenney
2017-06-15 17:34         ` [RFC] rcu: use killable versions of swait Paul E. McKenney

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=20170614230639.15079-1-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=paulmck@linux.vnet.ibm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.