* [PATCH 0/3] softirq: possible speedup and neatenings
@ 2013-11-17 9:55 Joe Perches
2013-11-17 9:55 ` [PATCH 1/3] softirq: Use ffs in __do_softirq Joe Perches
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Joe Perches @ 2013-11-17 9:55 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel
The first one boots, but it's barely tested.
Joe Perches (3):
softirq: Use ffs in __do_softirq
softirq: Convert printks to pr_<level>
softirq: Use const char * const for softirq_to_name, whitespace neatening
include/linux/interrupt.h | 2 +-
kernel/softirq.c | 75 +++++++++++++++++++++--------------------------
2 files changed, 35 insertions(+), 42 deletions(-)
--
1.8.1.2.459.gbcd45b4.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] softirq: Use ffs in __do_softirq 2013-11-17 9:55 [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches @ 2013-11-17 9:55 ` Joe Perches 2013-12-09 18:44 ` Frederic Weisbecker 2013-11-17 9:55 ` [PATCH 2/3] softirq: Convert printks to pr_<level> Joe Perches ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2013-11-17 9:55 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel Possible speed improvement of the __do_softirq function by using ffs instead of using a while loop with an & 1 test then single bit shift. Signed-off-by: Joe Perches <joe@perches.com> --- kernel/softirq.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index 11025cc..7be95d7 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -219,6 +219,7 @@ asmlinkage void __do_softirq(void) int cpu; unsigned long old_flags = current->flags; int max_restart = MAX_SOFTIRQ_RESTART; + int softirq_bit; /* * Mask out PF_MEMALLOC s current task context is borrowed for the @@ -242,30 +243,30 @@ restart: h = softirq_vec; - do { - if (pending & 1) { - unsigned int vec_nr = h - softirq_vec; - int prev_count = preempt_count(); - - kstat_incr_softirqs_this_cpu(vec_nr); - - trace_softirq_entry(vec_nr); - h->action(h); - trace_softirq_exit(vec_nr); - if (unlikely(prev_count != preempt_count())) { - printk(KERN_ERR "huh, entered softirq %u %s %p" - "with preempt_count %08x," - " exited with %08x?\n", vec_nr, - softirq_to_name[vec_nr], h->action, - prev_count, preempt_count()); - preempt_count_set(prev_count); - } + while ((softirq_bit = ffs(pending))) { + unsigned int vec_nr; + int prev_count; + + h += softirq_bit - 1; + + vec_nr = h - softirq_vec; + prev_count = preempt_count(); - rcu_bh_qs(cpu); + kstat_incr_softirqs_this_cpu(vec_nr); + + trace_softirq_entry(vec_nr); + h->action(h); + trace_softirq_exit(vec_nr); + if (unlikely(prev_count != preempt_count())) { + printk(KERN_ERR "huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n", + vec_nr, softirq_to_name[vec_nr], h->action, + prev_count, preempt_count()); + preempt_count_set(prev_count); } + rcu_bh_qs(cpu); h++; - pending >>= 1; - } while (pending); + pending >>= softirq_bit; + } local_irq_disable(); -- 1.8.1.2.459.gbcd45b4.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] softirq: Use ffs in __do_softirq 2013-11-17 9:55 ` [PATCH 1/3] softirq: Use ffs in __do_softirq Joe Perches @ 2013-12-09 18:44 ` Frederic Weisbecker 2013-12-09 18:56 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Frederic Weisbecker @ 2013-12-09 18:44 UTC (permalink / raw) To: Joe Perches; +Cc: Andrew Morton, linux-kernel On Sun, Nov 17, 2013 at 01:55:10AM -0800, Joe Perches wrote: > Possible speed improvement of the __do_softirq function by using ffs > instead of using a while loop with an & 1 test then single bit shift. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > kernel/softirq.c | 43 ++++++++++++++++++++++--------------------- > 1 file changed, 22 insertions(+), 21 deletions(-) > > diff --git a/kernel/softirq.c b/kernel/softirq.c > index 11025cc..7be95d7 100644 > --- a/kernel/softirq.c > +++ b/kernel/softirq.c > @@ -219,6 +219,7 @@ asmlinkage void __do_softirq(void) > int cpu; > unsigned long old_flags = current->flags; > int max_restart = MAX_SOFTIRQ_RESTART; > + int softirq_bit; > > /* > * Mask out PF_MEMALLOC s current task context is borrowed for the > @@ -242,30 +243,30 @@ restart: > > h = softirq_vec; > > - do { > - if (pending & 1) { > - unsigned int vec_nr = h - softirq_vec; > - int prev_count = preempt_count(); > - > - kstat_incr_softirqs_this_cpu(vec_nr); > - > - trace_softirq_entry(vec_nr); > - h->action(h); > - trace_softirq_exit(vec_nr); > - if (unlikely(prev_count != preempt_count())) { > - printk(KERN_ERR "huh, entered softirq %u %s %p" > - "with preempt_count %08x," > - " exited with %08x?\n", vec_nr, > - softirq_to_name[vec_nr], h->action, > - prev_count, preempt_count()); > - preempt_count_set(prev_count); > - } > + while ((softirq_bit = ffs(pending))) { > + unsigned int vec_nr; > + int prev_count; > + > + h += softirq_bit - 1; Perhaps using for_each_set_bit() would simplify that more? > + > + vec_nr = h - softirq_vec; > + prev_count = preempt_count(); > > - rcu_bh_qs(cpu); > + kstat_incr_softirqs_this_cpu(vec_nr); > + > + trace_softirq_entry(vec_nr); > + h->action(h); > + trace_softirq_exit(vec_nr); > + if (unlikely(prev_count != preempt_count())) { > + printk(KERN_ERR "huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n", > + vec_nr, softirq_to_name[vec_nr], h->action, > + prev_count, preempt_count()); > + preempt_count_set(prev_count); > } > + rcu_bh_qs(cpu); > h++; > - pending >>= 1; > - } while (pending); > + pending >>= softirq_bit; > + } > > local_irq_disable(); > > -- > 1.8.1.2.459.gbcd45b4.dirty > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] softirq: Use ffs in __do_softirq 2013-12-09 18:44 ` Frederic Weisbecker @ 2013-12-09 18:56 ` Joe Perches 2013-12-09 19:13 ` Frederic Weisbecker 0 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2013-12-09 18:56 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel On Mon, 2013-12-09 at 19:44 +0100, Frederic Weisbecker wrote: > On Sun, Nov 17, 2013 at 01:55:10AM -0800, Joe Perches wrote: > > Possible speed improvement of the __do_softirq function by using ffs > > instead of using a while loop with an & 1 test then single bit shift. [] > Perhaps using for_each_set_bit() would simplify that more? It might simplify the appearance of the code but it would/could expand the amount of generated code because for_each_set_bit uses an address_of(unsigned long) and the value tested is an unsigned int. extra dereferences, can't be in a register, etc... ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] softirq: Use ffs in __do_softirq 2013-12-09 18:56 ` Joe Perches @ 2013-12-09 19:13 ` Frederic Weisbecker 0 siblings, 0 replies; 10+ messages in thread From: Frederic Weisbecker @ 2013-12-09 19:13 UTC (permalink / raw) To: Joe Perches; +Cc: Andrew Morton, linux-kernel On Mon, Dec 09, 2013 at 10:56:46AM -0800, Joe Perches wrote: > On Mon, 2013-12-09 at 19:44 +0100, Frederic Weisbecker wrote: > > On Sun, Nov 17, 2013 at 01:55:10AM -0800, Joe Perches wrote: > > > Possible speed improvement of the __do_softirq function by using ffs > > > instead of using a while loop with an & 1 test then single bit shift. > [] > > Perhaps using for_each_set_bit() would simplify that more? > > It might simplify the appearance of the code but it > would/could expand the amount of generated code because > for_each_set_bit uses an address_of(unsigned long) and > the value tested is an unsigned int. > > extra dereferences, can't be in a register, etc... I'm not sure that would matter that much. But yeah it appears that find_first_bit/find_next_bit aren't even overriden in x86. So they are function calls. Although I guess that most of the time only one softirq is pending at a time. But anyway perhaps we want that path to stay very optimized, so you're patch look ok. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] softirq: Convert printks to pr_<level> 2013-11-17 9:55 [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches 2013-11-17 9:55 ` [PATCH 1/3] softirq: Use ffs in __do_softirq Joe Perches @ 2013-11-17 9:55 ` Joe Perches 2013-11-17 9:55 ` [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening Joe Perches 2013-12-09 17:22 ` [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches 3 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2013-11-17 9:55 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel Use a more current logging style. Signed-off-by: Joe Perches <joe@perches.com> --- kernel/softirq.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index 7be95d7..49a44cb 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -8,6 +8,8 @@ * Rewritten. Old one was good in 2.2, but in 2.3 it was immoral. --ANK (990903) */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include <linux/export.h> #include <linux/kernel_stat.h> #include <linux/interrupt.h> @@ -258,7 +260,7 @@ restart: h->action(h); trace_softirq_exit(vec_nr); if (unlikely(prev_count != preempt_count())) { - printk(KERN_ERR "huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n", + pr_err("huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n", vec_nr, softirq_to_name[vec_nr], h->action, prev_count, preempt_count()); preempt_count_set(prev_count); @@ -562,7 +564,7 @@ EXPORT_SYMBOL(tasklet_init); void tasklet_kill(struct tasklet_struct *t) { if (in_interrupt()) - printk("Attempt to kill tasklet from interrupt\n"); + pr_notice("Attempt to kill tasklet from interrupt\n"); while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) { do { -- 1.8.1.2.459.gbcd45b4.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening 2013-11-17 9:55 [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches 2013-11-17 9:55 ` [PATCH 1/3] softirq: Use ffs in __do_softirq Joe Perches 2013-11-17 9:55 ` [PATCH 2/3] softirq: Convert printks to pr_<level> Joe Perches @ 2013-11-17 9:55 ` Joe Perches 2013-12-24 15:19 ` Wang YanQing 2013-12-09 17:22 ` [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches 3 siblings, 1 reply; 10+ messages in thread From: Joe Perches @ 2013-11-17 9:55 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel Reduce data size a little. Reduce checkpatch noise. $ size kernel/softirq.o* text data bss dec hex filename 11554 6013 4008 21575 5447 kernel/softirq.o.new 11474 6093 4008 21575 5447 kernel/softirq.o.old Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/interrupt.h | 2 +- kernel/softirq.c | 28 +++++++++------------------- 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index db43b58..0053add 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -360,7 +360,7 @@ enum /* map softirq index to softirq name. update 'softirq_to_name' in * kernel/softirq.c when adding a new softirq. */ -extern char *softirq_to_name[NR_SOFTIRQS]; +extern const char * const softirq_to_name[NR_SOFTIRQS]; /* softirq mask and active fields moved to irq_cpustat_t in * asm/hardirq.h to get better cache usage. KAO diff --git a/kernel/softirq.c b/kernel/softirq.c index 49a44cb..c828df5 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -56,7 +56,7 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp DEFINE_PER_CPU(struct task_struct *, ksoftirqd); -char *softirq_to_name[NR_SOFTIRQS] = { +const char * const softirq_to_name[NR_SOFTIRQS] = { "HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "BLOCK_IOPOLL", "TASKLET", "SCHED", "HRTIMER", "RCU" }; @@ -128,7 +128,6 @@ void local_bh_disable(void) { __local_bh_disable(_RET_IP_, SOFTIRQ_DISABLE_OFFSET); } - EXPORT_SYMBOL(local_bh_disable); static void __local_bh_enable(unsigned int cnt) @@ -150,7 +149,6 @@ void _local_bh_enable(void) WARN_ON_ONCE(in_irq()); __local_bh_enable(SOFTIRQ_DISABLE_OFFSET); } - EXPORT_SYMBOL(_local_bh_enable); static inline void _local_bh_enable_ip(unsigned long ip) @@ -167,7 +165,7 @@ static inline void _local_bh_enable_ip(unsigned long ip) /* * Keep preemption disabled until we are done with * softirq processing: - */ + */ preempt_count_sub(SOFTIRQ_DISABLE_OFFSET - 1); if (unlikely(!in_interrupt() && local_softirq_pending())) { @@ -289,8 +287,6 @@ restart: tsk_restore_flags(current, old_flags, PF_MEMALLOC); } - - asmlinkage void do_softirq(void) { __u32 pending; @@ -430,8 +426,7 @@ void open_softirq(int nr, void (*action)(struct softirq_action *)) /* * Tasklets */ -struct tasklet_head -{ +struct tasklet_head { struct tasklet_struct *head; struct tasklet_struct **tail; }; @@ -450,7 +445,6 @@ void __tasklet_schedule(struct tasklet_struct *t) raise_softirq_irqoff(TASKLET_SOFTIRQ); local_irq_restore(flags); } - EXPORT_SYMBOL(__tasklet_schedule); void __tasklet_hi_schedule(struct tasklet_struct *t) @@ -464,7 +458,6 @@ void __tasklet_hi_schedule(struct tasklet_struct *t) raise_softirq_irqoff(HI_SOFTIRQ); local_irq_restore(flags); } - EXPORT_SYMBOL(__tasklet_hi_schedule); void __tasklet_hi_schedule_first(struct tasklet_struct *t) @@ -475,7 +468,6 @@ void __tasklet_hi_schedule_first(struct tasklet_struct *t) __this_cpu_write(tasklet_hi_vec.head, t); __raise_softirq_irqoff(HI_SOFTIRQ); } - EXPORT_SYMBOL(__tasklet_hi_schedule_first); static void tasklet_action(struct softirq_action *a) @@ -495,7 +487,8 @@ static void tasklet_action(struct softirq_action *a) if (tasklet_trylock(t)) { if (!atomic_read(&t->count)) { - if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)) + if (!test_and_clear_bit(TASKLET_STATE_SCHED, + &t->state)) BUG(); t->func(t->data); tasklet_unlock(t); @@ -530,7 +523,8 @@ static void tasklet_hi_action(struct softirq_action *a) if (tasklet_trylock(t)) { if (!atomic_read(&t->count)) { - if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)) + if (!test_and_clear_bit(TASKLET_STATE_SCHED, + &t->state)) BUG(); t->func(t->data); tasklet_unlock(t); @@ -548,7 +542,6 @@ static void tasklet_hi_action(struct softirq_action *a) } } - void tasklet_init(struct tasklet_struct *t, void (*func)(unsigned long), unsigned long data) { @@ -558,7 +551,6 @@ void tasklet_init(struct tasklet_struct *t, t->func = func; t->data = data; } - EXPORT_SYMBOL(tasklet_init); void tasklet_kill(struct tasklet_struct *t) @@ -574,7 +566,6 @@ void tasklet_kill(struct tasklet_struct *t) tasklet_unlock_wait(t); clear_bit(TASKLET_STATE_SCHED, &t->state); } - EXPORT_SYMBOL(tasklet_kill); /* @@ -724,9 +715,8 @@ static void takeover_tasklets(unsigned int cpu) } #endif /* CONFIG_HOTPLUG_CPU */ -static int cpu_callback(struct notifier_block *nfb, - unsigned long action, - void *hcpu) +static int cpu_callback(struct notifier_block *nfb, unsigned long action, + void *hcpu) { switch (action) { #ifdef CONFIG_HOTPLUG_CPU -- 1.8.1.2.459.gbcd45b4.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening 2013-11-17 9:55 ` [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening Joe Perches @ 2013-12-24 15:19 ` Wang YanQing 2013-12-24 15:27 ` Joe Perches 0 siblings, 1 reply; 10+ messages in thread From: Wang YanQing @ 2013-12-24 15:19 UTC (permalink / raw) To: Joe Perches; +Cc: Frederic Weisbecker, Andrew Morton, linux-kernel On Sun, Nov 17, 2013 at 01:55:12AM -0800, Joe Perches wrote: > Reduce data size a little. > Reduce checkpatch noise. > > $ size kernel/softirq.o* > text data bss dec hex filename > 11554 6013 4008 21575 5447 kernel/softirq.o.new > 11474 6093 4008 21575 5447 kernel/softirq.o.old Hi, could you tell me why this patch could reduce data size? Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening 2013-12-24 15:19 ` Wang YanQing @ 2013-12-24 15:27 ` Joe Perches 0 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2013-12-24 15:27 UTC (permalink / raw) To: Wang YanQing; +Cc: Frederic Weisbecker, Andrew Morton, linux-kernel On Tue, 2013-12-24 at 23:19 +0800, Wang YanQing wrote: > On Sun, Nov 17, 2013 at 01:55:12AM -0800, Joe Perches wrote: > > Reduce data size a little. > > Reduce checkpatch noise. > > > > $ size kernel/softirq.o* > > text data bss dec hex filename > > 11554 6013 4008 21575 5447 kernel/softirq.o.new > > 11474 6093 4008 21575 5447 kernel/softirq.o.old > > Hi, could you tell me why this patch could reduce data size? It moves the softirq_to_name array of 10 char * from data (non-const) to text (const). -char *softirq_to_name[NR_SOFTIRQS] = { +const char * const softirq_to_name[NR_SOFTIRQS] = { "HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "BLOCK_IOPOLL", "TASKLET", "SCHED", "HRTIMER", "RCU" }; Use objdump or "make kernel/softirq.lst" and inspect the object code produced to see for yourself. cheers, Joe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] softirq: possible speedup and neatenings 2013-11-17 9:55 [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches ` (2 preceding siblings ...) 2013-11-17 9:55 ` [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening Joe Perches @ 2013-12-09 17:22 ` Joe Perches 3 siblings, 0 replies; 10+ messages in thread From: Joe Perches @ 2013-12-09 17:22 UTC (permalink / raw) To: Frederic Weisbecker; +Cc: Andrew Morton, linux-kernel On Sun, 2013-11-17 at 01:55 -0800, Joe Perches wrote: > The first one boots, but it's barely tested. > > Joe Perches (3): > softirq: Use ffs in __do_softirq > softirq: Convert printks to pr_<level> > softirq: Use const char * const for softirq_to_name, whitespace neatening > > include/linux/interrupt.h | 2 +- > kernel/softirq.c | 75 +++++++++++++++++++++-------------------------- > 2 files changed, 35 insertions(+), 42 deletions(-) ping? ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-12-24 15:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-17 9:55 [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches 2013-11-17 9:55 ` [PATCH 1/3] softirq: Use ffs in __do_softirq Joe Perches 2013-12-09 18:44 ` Frederic Weisbecker 2013-12-09 18:56 ` Joe Perches 2013-12-09 19:13 ` Frederic Weisbecker 2013-11-17 9:55 ` [PATCH 2/3] softirq: Convert printks to pr_<level> Joe Perches 2013-11-17 9:55 ` [PATCH 3/3] softirq: Use const char * const for softirq_to_name, whitespace neatening Joe Perches 2013-12-24 15:19 ` Wang YanQing 2013-12-24 15:27 ` Joe Perches 2013-12-09 17:22 ` [PATCH 0/3] softirq: possible speedup and neatenings Joe Perches
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.