* [PATCH] rcu: optimize rcutiny @ 2011-04-28 5:23 Eric Dumazet 2011-04-29 7:54 ` Paul E. McKenney 0 siblings, 1 reply; 3+ messages in thread From: Eric Dumazet @ 2011-04-28 5:23 UTC (permalink / raw) To: Paul E. McKenney; +Cc: linux-kernel, Linus Torvalds rcu_sched_qs() currently calls local_irq_save()/local_irq_restore() up to three times. Remove irq masking from rcu_qsctr_help() / invoke_rcu_kthread() and do it once in rcu_sched_qs() / rcu_bh_qs() This generates smaller code as well. # size kernel/rcutiny.old.o kernel/rcutiny.new.o text data bss dec hex filename 2314 156 24 2494 9be kernel/rcutiny.old.o 2250 156 24 2430 97e kernel/rcutiny.new.o Fix an outdated comment for rcu_qsctr_help() Move invoke_rcu_kthread() definition before its use. Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- kernel/rcutiny.c | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c index 0c343b9..29eb349 100644 --- a/kernel/rcutiny.c +++ b/kernel/rcutiny.c @@ -40,7 +40,6 @@ static struct task_struct *rcu_kthread_task; static DECLARE_WAIT_QUEUE_HEAD(rcu_kthread_wq); static unsigned long have_rcu_kthread_work; -static void invoke_rcu_kthread(void); /* Forward declarations for rcutiny_plugin.h. */ struct rcu_ctrlblk; @@ -79,36 +78,45 @@ void rcu_exit_nohz(void) #endif /* #ifdef CONFIG_NO_HZ */ /* - * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc(). - * Also disable irqs to avoid confusion due to interrupt handlers + * Helper function for rcu_sched_qs() and rcu_bh_qs(). + * Also irqs are disabled to avoid confusion due to interrupt handlers * invoking call_rcu(). */ static int rcu_qsctr_help(struct rcu_ctrlblk *rcp) { - unsigned long flags; - - local_irq_save(flags); if (rcp->rcucblist != NULL && rcp->donetail != rcp->curtail) { rcp->donetail = rcp->curtail; - local_irq_restore(flags); return 1; } - local_irq_restore(flags); return 0; } /* + * Wake up rcu_kthread() to process callbacks now eligible for invocation + * or to boost readers. + */ +static void invoke_rcu_kthread(void) +{ + have_rcu_kthread_work = 1; + wake_up(&rcu_kthread_wq); +} + +/* * Record an rcu quiescent state. And an rcu_bh quiescent state while we * are at it, given that any rcu quiescent state is also an rcu_bh * quiescent state. Use "+" instead of "||" to defeat short circuiting. */ void rcu_sched_qs(int cpu) { + unsigned long flags; + + local_irq_save(flags); if (rcu_qsctr_help(&rcu_sched_ctrlblk) + rcu_qsctr_help(&rcu_bh_ctrlblk)) invoke_rcu_kthread(); + local_irq_restore(flags); } /* @@ -116,8 +124,12 @@ void rcu_sched_qs(int cpu) */ void rcu_bh_qs(int cpu) { + unsigned long flags; + + local_irq_save(flags); if (rcu_qsctr_help(&rcu_bh_ctrlblk)) invoke_rcu_kthread(); + local_irq_restore(flags); } /* @@ -208,20 +220,6 @@ static int rcu_kthread(void *arg) } /* - * Wake up rcu_kthread() to process callbacks now eligible for invocation - * or to boost readers. - */ -static void invoke_rcu_kthread(void) -{ - unsigned long flags; - - local_irq_save(flags); - have_rcu_kthread_work = 1; - wake_up(&rcu_kthread_wq); - local_irq_restore(flags); -} - -/* * Wait for a grace period to elapse. But it is illegal to invoke * synchronize_sched() from within an RCU read-side critical section. * Therefore, any legal call to synchronize_sched() is a quiescent ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] rcu: optimize rcutiny 2011-04-28 5:23 [PATCH] rcu: optimize rcutiny Eric Dumazet @ 2011-04-29 7:54 ` Paul E. McKenney 2011-04-29 7:55 ` Paul E. McKenney 0 siblings, 1 reply; 3+ messages in thread From: Paul E. McKenney @ 2011-04-29 7:54 UTC (permalink / raw) To: Eric Dumazet; +Cc: linux-kernel, Linus Torvalds On Thu, Apr 28, 2011 at 07:23:45AM +0200, Eric Dumazet wrote: > rcu_sched_qs() currently calls local_irq_save()/local_irq_restore() up > to three times. > > Remove irq masking from rcu_qsctr_help() / invoke_rcu_kthread() > and do it once in rcu_sched_qs() / rcu_bh_qs() > > This generates smaller code as well. > > # size kernel/rcutiny.old.o kernel/rcutiny.new.o > text data bss dec hex filename > 2314 156 24 2494 9be kernel/rcutiny.old.o > 2250 156 24 2430 97e kernel/rcutiny.new.o > > Fix an outdated comment for rcu_qsctr_help() > Move invoke_rcu_kthread() definition before its use. Looks very nice! In theory, this does lengthen the time during which interrupts are disabled, but in practice I believe that that this would not be measurable. Adding Thomas on CC in case I am mistaken about the effect of longer irq-disable regions. In the meantime, I have queued this, and either way, thank you, Eric! Thanx, Paul > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > kernel/rcutiny.c | 42 ++++++++++++++++++++---------------------- > 1 file changed, 20 insertions(+), 22 deletions(-) > > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c > index 0c343b9..29eb349 100644 > --- a/kernel/rcutiny.c > +++ b/kernel/rcutiny.c > @@ -40,7 +40,6 @@ > static struct task_struct *rcu_kthread_task; > static DECLARE_WAIT_QUEUE_HEAD(rcu_kthread_wq); > static unsigned long have_rcu_kthread_work; > -static void invoke_rcu_kthread(void); > > /* Forward declarations for rcutiny_plugin.h. */ > struct rcu_ctrlblk; > @@ -79,36 +78,45 @@ void rcu_exit_nohz(void) > #endif /* #ifdef CONFIG_NO_HZ */ > > /* > - * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc(). > - * Also disable irqs to avoid confusion due to interrupt handlers > + * Helper function for rcu_sched_qs() and rcu_bh_qs(). > + * Also irqs are disabled to avoid confusion due to interrupt handlers > * invoking call_rcu(). > */ > static int rcu_qsctr_help(struct rcu_ctrlblk *rcp) > { > - unsigned long flags; > - > - local_irq_save(flags); > if (rcp->rcucblist != NULL && > rcp->donetail != rcp->curtail) { > rcp->donetail = rcp->curtail; > - local_irq_restore(flags); > return 1; > } > - local_irq_restore(flags); > > return 0; > } > > /* > + * Wake up rcu_kthread() to process callbacks now eligible for invocation > + * or to boost readers. > + */ > +static void invoke_rcu_kthread(void) > +{ > + have_rcu_kthread_work = 1; > + wake_up(&rcu_kthread_wq); > +} > + > +/* > * Record an rcu quiescent state. And an rcu_bh quiescent state while we > * are at it, given that any rcu quiescent state is also an rcu_bh > * quiescent state. Use "+" instead of "||" to defeat short circuiting. > */ > void rcu_sched_qs(int cpu) > { > + unsigned long flags; > + > + local_irq_save(flags); > if (rcu_qsctr_help(&rcu_sched_ctrlblk) + > rcu_qsctr_help(&rcu_bh_ctrlblk)) > invoke_rcu_kthread(); > + local_irq_restore(flags); > } > > /* > @@ -116,8 +124,12 @@ void rcu_sched_qs(int cpu) > */ > void rcu_bh_qs(int cpu) > { > + unsigned long flags; > + > + local_irq_save(flags); > if (rcu_qsctr_help(&rcu_bh_ctrlblk)) > invoke_rcu_kthread(); > + local_irq_restore(flags); > } > > /* > @@ -208,20 +220,6 @@ static int rcu_kthread(void *arg) > } > > /* > - * Wake up rcu_kthread() to process callbacks now eligible for invocation > - * or to boost readers. > - */ > -static void invoke_rcu_kthread(void) > -{ > - unsigned long flags; > - > - local_irq_save(flags); > - have_rcu_kthread_work = 1; > - wake_up(&rcu_kthread_wq); > - local_irq_restore(flags); > -} > - > -/* > * Wait for a grace period to elapse. But it is illegal to invoke > * synchronize_sched() from within an RCU read-side critical section. > * Therefore, any legal call to synchronize_sched() is a quiescent > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rcu: optimize rcutiny 2011-04-29 7:54 ` Paul E. McKenney @ 2011-04-29 7:55 ` Paul E. McKenney 0 siblings, 0 replies; 3+ messages in thread From: Paul E. McKenney @ 2011-04-29 7:55 UTC (permalink / raw) To: Eric Dumazet; +Cc: linux-kernel, Linus Torvalds, tglx This time actually adding Thomas to CC... :-/ Thanx, Paul On Fri, Apr 29, 2011 at 12:54:32AM -0700, Paul E. McKenney wrote: > On Thu, Apr 28, 2011 at 07:23:45AM +0200, Eric Dumazet wrote: > > rcu_sched_qs() currently calls local_irq_save()/local_irq_restore() up > > to three times. > > > > Remove irq masking from rcu_qsctr_help() / invoke_rcu_kthread() > > and do it once in rcu_sched_qs() / rcu_bh_qs() > > > > This generates smaller code as well. > > > > # size kernel/rcutiny.old.o kernel/rcutiny.new.o > > text data bss dec hex filename > > 2314 156 24 2494 9be kernel/rcutiny.old.o > > 2250 156 24 2430 97e kernel/rcutiny.new.o > > > > Fix an outdated comment for rcu_qsctr_help() > > Move invoke_rcu_kthread() definition before its use. > > Looks very nice! In theory, this does lengthen the time during which > interrupts are disabled, but in practice I believe that that this would > not be measurable. Adding Thomas on CC in case I am mistaken about > the effect of longer irq-disable regions. > > In the meantime, I have queued this, and either way, thank you, Eric! > > Thanx, Paul > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > --- > > kernel/rcutiny.c | 42 ++++++++++++++++++++---------------------- > > 1 file changed, 20 insertions(+), 22 deletions(-) > > > > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c > > index 0c343b9..29eb349 100644 > > --- a/kernel/rcutiny.c > > +++ b/kernel/rcutiny.c > > @@ -40,7 +40,6 @@ > > static struct task_struct *rcu_kthread_task; > > static DECLARE_WAIT_QUEUE_HEAD(rcu_kthread_wq); > > static unsigned long have_rcu_kthread_work; > > -static void invoke_rcu_kthread(void); > > > > /* Forward declarations for rcutiny_plugin.h. */ > > struct rcu_ctrlblk; > > @@ -79,36 +78,45 @@ void rcu_exit_nohz(void) > > #endif /* #ifdef CONFIG_NO_HZ */ > > > > /* > > - * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc(). > > - * Also disable irqs to avoid confusion due to interrupt handlers > > + * Helper function for rcu_sched_qs() and rcu_bh_qs(). > > + * Also irqs are disabled to avoid confusion due to interrupt handlers > > * invoking call_rcu(). > > */ > > static int rcu_qsctr_help(struct rcu_ctrlblk *rcp) > > { > > - unsigned long flags; > > - > > - local_irq_save(flags); > > if (rcp->rcucblist != NULL && > > rcp->donetail != rcp->curtail) { > > rcp->donetail = rcp->curtail; > > - local_irq_restore(flags); > > return 1; > > } > > - local_irq_restore(flags); > > > > return 0; > > } > > > > /* > > + * Wake up rcu_kthread() to process callbacks now eligible for invocation > > + * or to boost readers. > > + */ > > +static void invoke_rcu_kthread(void) > > +{ > > + have_rcu_kthread_work = 1; > > + wake_up(&rcu_kthread_wq); > > +} > > + > > +/* > > * Record an rcu quiescent state. And an rcu_bh quiescent state while we > > * are at it, given that any rcu quiescent state is also an rcu_bh > > * quiescent state. Use "+" instead of "||" to defeat short circuiting. > > */ > > void rcu_sched_qs(int cpu) > > { > > + unsigned long flags; > > + > > + local_irq_save(flags); > > if (rcu_qsctr_help(&rcu_sched_ctrlblk) + > > rcu_qsctr_help(&rcu_bh_ctrlblk)) > > invoke_rcu_kthread(); > > + local_irq_restore(flags); > > } > > > > /* > > @@ -116,8 +124,12 @@ void rcu_sched_qs(int cpu) > > */ > > void rcu_bh_qs(int cpu) > > { > > + unsigned long flags; > > + > > + local_irq_save(flags); > > if (rcu_qsctr_help(&rcu_bh_ctrlblk)) > > invoke_rcu_kthread(); > > + local_irq_restore(flags); > > } > > > > /* > > @@ -208,20 +220,6 @@ static int rcu_kthread(void *arg) > > } > > > > /* > > - * Wake up rcu_kthread() to process callbacks now eligible for invocation > > - * or to boost readers. > > - */ > > -static void invoke_rcu_kthread(void) > > -{ > > - unsigned long flags; > > - > > - local_irq_save(flags); > > - have_rcu_kthread_work = 1; > > - wake_up(&rcu_kthread_wq); > > - local_irq_restore(flags); > > -} > > - > > -/* > > * Wait for a grace period to elapse. But it is illegal to invoke > > * synchronize_sched() from within an RCU read-side critical section. > > * Therefore, any legal call to synchronize_sched() is a quiescent > > > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-29 11:37 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-28 5:23 [PATCH] rcu: optimize rcutiny Eric Dumazet 2011-04-29 7:54 ` Paul E. McKenney 2011-04-29 7:55 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox