linux-kernel - Re: [PATCH,RFC] Add call_rcu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 24 Mar 2008 09:59:53 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu,
	akpm@...ux-foundation.org, hch@...radead.org, mmlnx@...ibm.com,
	dipankar@...ibm.com, dsmith@...hat.com, rostedt@...dmis.org,
	adrian.bunk@...ial.fi, a.p.zijlstra@...llo.nl, ego@...ibm.com,
	niv@...ibm.com, dvhltc@...ibm.com, rusty@....ibm.com,
	jkenisto@...ux.vnet.ibm.com, oleg@...sign.ru
Subject: Re: [PATCH,RFC] Add call_rcu_sched()

* Paul E. McKenney (paulmck@...ux.vnet.ibm.com) wrote:
> On Mon, Mar 24, 2008 at 01:06:53AM -0400, Mathieu Desnoyers wrote:
> > * Paul E. McKenney (paulmck@...ux.vnet.ibm.com) wrote:
[...]
> > > o	Interaction of this patch with CPU hotplug should be viewed
> > > 	with great suspicion.
> > 
> > Fix call_rcu_sched wait
> 
> There are definitely some problems here...  Though I am seeing them
> in the sched_setaffinity() call rather than in the wait processing.
> 

Sorry for the misleading line : "Fix call_rcu_sched wait" was the title
of the patch addressing the rcu_sched_grace:924 blocked ... problem below.

> > > o If there are no synchronize_sched() calls for more than two
> > >   minutes, one can see messages of the form "INFO: task
> > >   rcu_sched_grace:924 blocked for more than 120 seconds."
> > >   Any thoughts on how to avoid this message?  Should I be using
> > >   something other than __wait_event() and wake_up(), which sleep
> > >   uninterruptibly, thus triggering this message?
> > > 
> > 
[...]
> > Could you use __wait_event_interruptible and wake_up_interruptible
> > instead ? softlockup.c only seems to complain when uninterruptible tasks
> > are not scheduled for 2 minutes. I guess that when we receive a signal
> > we could simply go through another loop.
> 
> I will give these a try.
> 
> > +		ret = 0;
> > +		__wait_event_interruptible(rcu_ctrlblk.sched_wq,
> > +			     rcu_ctrlblk.sched_sleep != rcu_sched_sleeping,
> > +			     ret);
> 
> Don't we have to do something here to clear signal state if we are
> ever to block again?  Maybe something like the following?
> 
> 		flush_signals(current):
> 
> Or am I missing something?
> 

Good point, I would add
if (ret < 0)
  flush_signals(current);

[...]
> > 
> > That's always good :)
> 
> Fixing the bug or losing track?  ;-)
> 

Fixing it of course :)

New version of the fix-call-rcu-sched-wait.patch file below.

Mathieu


Fix call_rcu_sched wait

> o If there are no synchronize_sched() calls for more than two
>   minutes, one can see messages of the form "INFO: task
>   rcu_sched_grace:924 blocked for more than 120 seconds."
>   Any thoughts on how to avoid this message?  Should I be using
>   something other than __wait_event() and wake_up(), which sleep
>   uninterruptibly, thus triggering this message?
> 

Could you use __wait_event_interruptible and wake_up_interruptible
instead ? softlockup.c only seems to complain when uninterruptible tasks
are not scheduled for 2 minutes. I guess that when we receive a signal
we could simply go through another loop.

- Changelog
Reset signal state upon wakeup.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
---
 kernel/rcupreempt.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux-2.6-lttng/kernel/rcupreempt.c
===================================================================
--- linux-2.6-lttng.orig/kernel/rcupreempt.c	2008-03-24 00:26:27.000000000 -0400
+++ linux-2.6-lttng/kernel/rcupreempt.c	2008-03-24 09:57:28.000000000 -0400
@@ -1074,7 +1074,7 @@ void call_rcu_sched(struct rcu_head *hea
 		rcu_ctrlblk.sched_sleep = rcu_sched_not_sleeping;
 		spin_unlock_irqrestore(&rcu_ctrlblk.schedlock, flags);
 		if (wake_gp)
-			wake_up(&rcu_ctrlblk.sched_wq);
+			wake_up_interruptible(&rcu_ctrlblk.sched_wq);
 	}
 }
 EXPORT_SYMBOL_GPL(call_rcu_sched);
@@ -1097,6 +1097,7 @@ rcu_sched_grace_period(void *arg)
 	int couldsleep;		/* might sleep after current pass. */
 	int couldsleepnext = 0; /* might sleep after next pass. */
 	int cpu;
+	int ret;
 	long err;
 	unsigned long flags;
 	int needsoftirq;
@@ -1242,8 +1243,12 @@ retry:
 
 		rcu_ctrlblk.sched_sleep = rcu_sched_sleeping;
 		spin_unlock_irqrestore(&rcu_ctrlblk.schedlock, flags);
-		__wait_event(rcu_ctrlblk.sched_wq,
-			     rcu_ctrlblk.sched_sleep != rcu_sched_sleeping);
+		ret = 0;
+		__wait_event_interruptible(rcu_ctrlblk.sched_wq,
+			     rcu_ctrlblk.sched_sleep != rcu_sched_sleeping,
+			     ret);
+		if (ret < 0)
+			flush_signals(current);
 		couldsleepnext = 0;
 
 	} while (!kthread_should_stop());

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/