linux-kernel - Re: [PATCH tip/core/rcu 13/13] rcu: Shrink TINY

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] [day] [month] [year] [list]

Message-ID: <20130521214832.GC11463@jtriplet-mobl1>
Date:	Tue, 21 May 2013 14:48:32 -0700
From:	Josh Triplett <josh@...htriplett.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, laijs@...fujitsu.com,
	dipankar@...ibm.com, akpm@...ux-foundation.org,
	mathieu.desnoyers@...ymtl.ca, niv@...ibm.com, tglx@...utronix.de,
	peterz@...radead.org, rostedt@...dmis.org, Valdis.Kletnieks@...edu,
	dhowells@...hat.com, edumazet@...gle.com, darren@...art.com,
	fweisbec@...il.com, sbw@....edu
Subject: Re: [PATCH tip/core/rcu 13/13] rcu: Shrink TINY_RCU by reworking
 CPU-stall ifdefs

On Tue, May 21, 2013 at 02:09:57PM -0700, Paul E. McKenney wrote:
> On Tue, May 21, 2013 at 10:31:41AM -0700, Josh Triplett wrote:
> > On Mon, May 20, 2013 at 07:58:20AM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > > 
> > > TINY_RCU's reset_cpu_stall_ticks() and check_cpu_stalls() functions
> > > are defined unconditionally, and are empty functions if CONFIG_RCU_TRACE
> > > is disabled (which in turns disables detection of RCU CPU stalls).
> > > These empty functions can add a bit of bloat to TINY_RCU, so this
> > > commit reworks the ifdefs so that these functions are defined only
> > > if they actually do something.
> > > 
> > > Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > 
> > Strange.  Both of these functions are static, and called in only one
> > place, so shouldn't the compiler inline them (and thus throw them away
> > completely when empty)?  Why does it not do so?
> 
> Good point for most architectures.  But given Tiny RCU's purpose, it
> should allow for stupid compilers.  And I do need to fix the commit
> message to call out the other benefit, namely saving a couple of
> lines of source code.

Regarding function inlining, at least, GCC is usually equally stupid on
all architectures.  I'd be quite surprised if any architecture actually
emits code for these functions.  You could also try changing them to
"static inline", in which case GCC should *definitely* never emit code
for them when empty.

- Josh Triplett

> 							Thanx, Paul
> 
> > - Josh Triplett
> > 
> > > ---
> > >  kernel/rcutiny.c        | 4 ++--
> > >  kernel/rcutiny_plugin.h | 6 ++----
> > >  2 files changed, 4 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> > > index 4adc9e2..aa34411 100644
> > > --- a/kernel/rcutiny.c
> > > +++ b/kernel/rcutiny.c
> > > @@ -204,7 +204,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> > >   */
> > >  static int rcu_qsctr_help(struct rcu_ctrlblk *rcp)
> > >  {
> > > -	reset_cpu_stall_ticks(rcp);
> > > +	RCU_TRACE(reset_cpu_stall_ticks(rcp));
> > >  	if (rcp->rcucblist != NULL &&
> > >  	    rcp->donetail != rcp->curtail) {
> > >  		rcp->donetail = rcp->curtail;
> > > @@ -251,7 +251,7 @@ void rcu_bh_qs(int cpu)
> > >   */
> > >  void rcu_check_callbacks(int cpu, int user)
> > >  {
> > > -	check_cpu_stalls();
> > > +	RCU_TRACE(check_cpu_stalls());
> > >  	if (user || rcu_is_cpu_rrupt_from_idle())
> > >  		rcu_sched_qs(cpu);
> > >  	else if (!in_softirq())
> > > diff --git a/kernel/rcutiny_plugin.h b/kernel/rcutiny_plugin.h
> > > index 65ef180..0cd385a 100644
> > > --- a/kernel/rcutiny_plugin.h
> > > +++ b/kernel/rcutiny_plugin.h
> > > @@ -158,15 +158,11 @@ static void check_cpu_stall(struct rcu_ctrlblk *rcp)
> > >  		rcp->jiffies_stall = jiffies + rcu_jiffies_till_stall_check();
> > >  }
> > >  
> > > -#endif /* #ifdef CONFIG_RCU_TRACE */
> > > -
> > >  static void reset_cpu_stall_ticks(struct rcu_ctrlblk *rcp)
> > >  {
> > > -#ifdef CONFIG_RCU_TRACE
> > >  	rcp->ticks_this_gp = 0;
> > >  	rcp->gp_start = jiffies;
> > >  	rcp->jiffies_stall = jiffies + rcu_jiffies_till_stall_check();
> > > -#endif /* #ifdef CONFIG_RCU_TRACE */
> > >  }
> > >  
> > >  static void check_cpu_stalls(void)
> > > @@ -174,3 +170,5 @@ static void check_cpu_stalls(void)
> > >  	RCU_TRACE(check_cpu_stall(&rcu_bh_ctrlblk));
> > >  	RCU_TRACE(check_cpu_stall(&rcu_sched_ctrlblk));
> > >  }
> > > +
> > > +#endif /* #ifdef CONFIG_RCU_TRACE */
> > > -- 
> > > 1.8.1.5
> > > 
> > --
> > 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/
> > 
> 
--
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/