linux-kernel - Re: [PATCH tip/core/rcu 06/10] trace: Eliminate cond_resched_rcu_qs() in favor of cond

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:   Wed, 28 Feb 2018 17:21:44 -0800
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, mingo@...nel.org,
        jiangshanlai@...il.com, dipankar@...ibm.com,
        akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
        josh@...htriplett.org, tglx@...utronix.de, peterz@...radead.org,
        dhowells@...hat.com, edumazet@...gle.com, fweisbec@...il.com,
        oleg@...hat.com, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH tip/core/rcu 06/10] trace: Eliminate
 cond_resched_rcu_qs() in favor of cond_resched()

On Wed, Feb 28, 2018 at 06:12:52PM -0500, Steven Rostedt wrote:
> On Tue, 27 Feb 2018 07:36:46 -0800
> "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -1589,6 +1589,12 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
> > > >   */
> > > >  #ifndef CONFIG_PREEMPT
> > > >  extern int _cond_resched(void);
> > > > +#elif defined(CONFIG_TASKS_RCU)
> > > > +static inline int _cond_resched(void)
> > > > +{
> > > > +	rcu_note_voluntary_context_switch(current);
> > > > +	return 0;
> > > > +}
> > > >  #else
> > > >  static inline int _cond_resched(void) { return 0; }
> > > >  #endif  
> > > 
> > > 
> > > This does work, but so does the below, without causing cond_resched()
> > > from being something other than a nop of CONFIG_PREEMPT.  
> > 
> > True, but based on the cond_resched_rcu_qs() experience, I bet that
> > trace_benchmark.c won't be the only place needing help.
> 
> Perhaps, still think this is a special case. That said, perhaps
> cond_resched isn't done in critical locations as it's a place that is
> explicitly stating that it's OK to schedule.

Building on your second sentence, when you are running a non-production
stress test, adding an extra function call and conditional branch to
cond_resched() should not be a problem.

So how about the (still untested) patch below?

							Thanx, Paul

------------------------------------------------------------------------

commit e9a6ea9fc2542459f9a63cf2b3a0264d09fbc266
Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Date:   Sun Feb 25 10:40:44 2018 -0800

    EXP sched: Make non-production PREEMPT cond_resched() help Tasks RCU
    
    In CONFIG_PREEMPT=y kernels, cond_resched() is a complete no-op, and
    thus cannot help advance Tasks-RCU grace periods.  However, such grace
    periods are only an issue in non-production benchmarking runs of the
    Linux kernel.  This commit therefore makes cond_resched() invoke
    rcu_note_voluntary_context_switch() for kernels implementing Tasks RCU
    even in CONFIG_PREEMPT=y kernels.
    
    Reported-by: Steven Rostedt <rostedt@...dmis.org>
    Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>

diff --git a/include/linux/sched.h b/include/linux/sched.h
index b161ef8a902e..970dadefb86f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1589,6 +1589,12 @@ static inline int test_tsk_need_resched(struct task_struct *tsk)
  */
 #ifndef CONFIG_PREEMPT
 extern int _cond_resched(void);
+#elif defined(CONFIG_TRACEPOINT_BENCHMARK)
+static inline int _cond_resched(void)
+{
+	rcu_note_voluntary_context_switch(current);
+	return 0;
+}
 #else
 static inline int _cond_resched(void) { return 0; }
 #endif