From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752732AbdFNXGq (ORCPT ); Wed, 14 Jun 2017 19:06:46 -0400 Received: from mail.kernel.org ([198.145.29.99]:50970 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752702AbdFNXGp (ORCPT ); Wed, 14 Jun 2017 19:06:45 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 03991219A8 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mcgrof@kernel.org From: "Luis R. Rodriguez" 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" Subject: [RFC] rcu: use killable versions of swait Date: Wed, 14 Jun 2017 16:06:39 -0700 Message-Id: <20170614230639.15079-1-mcgrof@kernel.org> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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