* [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
* [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 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
* 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
* 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
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.