* [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code
@ 2013-06-09 12:30 Chen Gang
2013-06-10 12:25 ` Paul E. McKenney
2013-06-10 14:15 ` Thomas Gleixner
0 siblings, 2 replies; 5+ messages in thread
From: Chen Gang @ 2013-06-09 12:30 UTC (permalink / raw)
To: Frederic Weisbecker, Thomas Gleixner, Sedat Dilek, Paul McKenney
Cc: Andrew Morton, Linus Torvalds, linux-kernel@vger.kernel.org
After finish the internal 'while', need not test TASKLET_STATE_SCHED
again, so looping back to outside 'while' is only for set_bit().
When use 'if' and set_bit() instead of 'while', it will save at least
one running conditional instruction, and also will be clearer for readers
(although the binary size will be a little bigger).
The related patch is "1da177e Linux-2.6.12-rc2"
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/softirq.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index a5f8836..52da25f 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -540,10 +540,11 @@ void tasklet_kill(struct tasklet_struct *t)
if (in_interrupt())
printk("Attempt to kill tasklet from interrupt\n");
- while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
+ if (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
do {
yield();
} while (test_bit(TASKLET_STATE_SCHED, &t->state));
+ set_bit(TASKLET_STATE_SCHED, &t->state);
}
tasklet_unlock_wait(t);
clear_bit(TASKLET_STATE_SCHED, &t->state);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code
2013-06-09 12:30 [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code Chen Gang
@ 2013-06-10 12:25 ` Paul E. McKenney
2013-06-13 2:08 ` Chen Gang
2013-06-10 14:15 ` Thomas Gleixner
1 sibling, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2013-06-10 12:25 UTC (permalink / raw)
To: Chen Gang
Cc: Frederic Weisbecker, Thomas Gleixner, Sedat Dilek, Andrew Morton,
Linus Torvalds, linux-kernel@vger.kernel.org
On Sun, Jun 09, 2013 at 08:30:19PM +0800, Chen Gang wrote:
>
> After finish the internal 'while', need not test TASKLET_STATE_SCHED
> again, so looping back to outside 'while' is only for set_bit().
>
> When use 'if' and set_bit() instead of 'while', it will save at least
> one running conditional instruction, and also will be clearer for readers
> (although the binary size will be a little bigger).
>
> The related patch is "1da177e Linux-2.6.12-rc2"
>
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> kernel/softirq.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index a5f8836..52da25f 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -540,10 +540,11 @@ void tasklet_kill(struct tasklet_struct *t)
> if (in_interrupt())
> printk("Attempt to kill tasklet from interrupt\n");
>
> - while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
> + if (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
> do {
> yield();
> } while (test_bit(TASKLET_STATE_SCHED, &t->state));
> + set_bit(TASKLET_STATE_SCHED, &t->state);
This replaces an atomic test-and-set with two operations, a test and
a set. Is this safe?
Thanx, Paul
> }
> tasklet_unlock_wait(t);
> clear_bit(TASKLET_STATE_SCHED, &t->state);
> --
> 1.7.7.6
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code
2013-06-10 12:25 ` Paul E. McKenney
@ 2013-06-13 2:08 ` Chen Gang
0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-06-13 2:08 UTC (permalink / raw)
To: paulmck
Cc: Frederic Weisbecker, Thomas Gleixner, Sedat Dilek, Andrew Morton,
Linus Torvalds, linux-kernel@vger.kernel.org
On 06/10/2013 08:25 PM, Paul E. McKenney wrote:
> On Sun, Jun 09, 2013 at 08:30:19PM +0800, Chen Gang wrote:
>> >
>> > After finish the internal 'while', need not test TASKLET_STATE_SCHED
>> > again, so looping back to outside 'while' is only for set_bit().
>> >
>> > When use 'if' and set_bit() instead of 'while', it will save at least
>> > one running conditional instruction, and also will be clearer for readers
>> > (although the binary size will be a little bigger).
>> >
>> > The related patch is "1da177e Linux-2.6.12-rc2"
>> >
>> >
>> > Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> > ---
>> > kernel/softirq.c | 3 ++-
>> > 1 files changed, 2 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/kernel/softirq.c b/kernel/softirq.c
>> > index a5f8836..52da25f 100644
>> > --- a/kernel/softirq.c
>> > +++ b/kernel/softirq.c
>> > @@ -540,10 +540,11 @@ void tasklet_kill(struct tasklet_struct *t)
>> > if (in_interrupt())
>> > printk("Attempt to kill tasklet from interrupt\n");
>> >
>> > - while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
>> > + if (test_and_set_bit(TASKLET_STATE_SCHED, &t->state)) {
>> > do {
>> > yield();
>> > } while (test_bit(TASKLET_STATE_SCHED, &t->state));
>> > + set_bit(TASKLET_STATE_SCHED, &t->state);
> This replaces an atomic test-and-set with two operations, a test and
> a set. Is this safe?
Oh, it seems not safe, at least it is not the original author's willing.
It is my fault, and also sorry for replying late.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code
2013-06-09 12:30 [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code Chen Gang
2013-06-10 12:25 ` Paul E. McKenney
@ 2013-06-10 14:15 ` Thomas Gleixner
2013-06-13 2:10 ` Chen Gang
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2013-06-10 14:15 UTC (permalink / raw)
To: Chen Gang
Cc: Frederic Weisbecker, Sedat Dilek, Paul McKenney, Andrew Morton,
Linus Torvalds, linux-kernel@vger.kernel.org
On Sun, 9 Jun 2013, Chen Gang wrote:
> After finish the internal 'while', need not test TASKLET_STATE_SCHED
> again, so looping back to outside 'while' is only for set_bit().
>
> When use 'if' and set_bit() instead of 'while', it will save at least
> one running conditional instruction, and also will be clearer for readers
> (although the binary size will be a little bigger).
And by doing that you break the atomicity of test_and_set_bit. There
is a good reason why this is an atomic operation and why the code is
written as is.
> The related patch is "1da177e Linux-2.6.12-rc2"
How is that patch related to the problem?
Thanks
tglx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code
2013-06-10 14:15 ` Thomas Gleixner
@ 2013-06-13 2:10 ` Chen Gang
0 siblings, 0 replies; 5+ messages in thread
From: Chen Gang @ 2013-06-13 2:10 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Frederic Weisbecker, Sedat Dilek, Paul McKenney, Andrew Morton,
Linus Torvalds, linux-kernel@vger.kernel.org
On 06/10/2013 10:15 PM, Thomas Gleixner wrote:
> On Sun, 9 Jun 2013, Chen Gang wrote:
>> > After finish the internal 'while', need not test TASKLET_STATE_SCHED
>> > again, so looping back to outside 'while' is only for set_bit().
>> >
>> > When use 'if' and set_bit() instead of 'while', it will save at least
>> > one running conditional instruction, and also will be clearer for readers
>> > (although the binary size will be a little bigger).
> And by doing that you break the atomicity of test_and_set_bit. There
> is a good reason why this is an atomic operation and why the code is
> written as is.
>
OK, thanks. it is my fault (and also sorry for replying late).
>> > The related patch is "1da177e Linux-2.6.12-rc2"
> How is that patch related to the problem?
Because the modification since Linux-2.6.12-rc2.
Thanks.
--
Chen Gang
Asianux Corporation
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-06-13 2:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-09 12:30 [PATCH] kernel/softirq.c: delete 'while' looping to improve a little performance and beautify code Chen Gang
2013-06-10 12:25 ` Paul E. McKenney
2013-06-13 2:08 ` Chen Gang
2013-06-10 14:15 ` Thomas Gleixner
2013-06-13 2:10 ` Chen Gang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox