* [PATCH]sched: completion: use bool in try_wait_for_completion
@ 2018-02-21 12:54 gaurav jindal
2018-02-21 13:28 ` Peter Zijlstra
2018-03-09 9:06 ` [tip:sched/core] sched/completions: Use bool in try_wait_for_completion() tip-bot for gaurav jindal
0 siblings, 2 replies; 7+ messages in thread
From: gaurav jindal @ 2018-02-21 12:54 UTC (permalink / raw)
To: peterz, mingo; +Cc: linux-kernel
Variable ret in try_wait_for_completion can have only true/false values. Since
the return type of the function is also bool, variable ret should have data
type as bool in place of int.
Moreover, the size of bool in many platforms is 1 byte whereas size of int is
4 bytes(though architecture dependent). Modifying the data type reduces the
size consumed for the stack.
Signed-off-by: Gaurav Jindal<gauravjindal1104@gmail.com>
---
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 0926aef..3e15e8d 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -283,7 +283,7 @@ int __sched wait_for_completion_killable(struct completion *x)
bool try_wait_for_completion(struct completion *x)
{
unsigned long flags;
- int ret = 1;
+ bool ret = true;
/*
* Since x->done will need to be locked only
@@ -292,11 +292,11 @@ bool try_wait_for_completion(struct completion *x)
* return early in the blocking case.
*/
if (!READ_ONCE(x->done))
- return 0;
+ return false;
spin_lock_irqsave(&x->wait.lock, flags);
if (!x->done)
- ret = 0;
+ ret = false;
else if (x->done != UINT_MAX)
x->done--;
spin_unlock_irqrestore(&x->wait.lock, flags);
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH]sched: completion: use bool in try_wait_for_completion
2018-02-21 12:54 [PATCH]sched: completion: use bool in try_wait_for_completion gaurav jindal
@ 2018-02-21 13:28 ` Peter Zijlstra
2018-02-21 13:49 ` gaurav jindal
2018-03-09 9:06 ` [tip:sched/core] sched/completions: Use bool in try_wait_for_completion() tip-bot for gaurav jindal
1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2018-02-21 13:28 UTC (permalink / raw)
To: gaurav jindal; +Cc: mingo, linux-kernel
On Wed, Feb 21, 2018 at 06:24:07PM +0530, gaurav jindal wrote:
> Variable ret in try_wait_for_completion can have only true/false values. Since
> the return type of the function is also bool, variable ret should have data
> type as bool in place of int.
Fair enough.
> Moreover, the size of bool in many platforms is 1 byte whereas size of int is
> 4 bytes(though architecture dependent). Modifying the data type reduces the
> size consumed for the stack.
Absolutely 0 difference in generated assembly here on x86_64-defconfig
gcc Debian 7.2.0-20.
$ objdump -dr defconfig-build/kernel/sched/completion.o | awk '/^$/ {p=0} /<try_wait_for_completion>:$/ {p=1} {if (p) print $0}'
0000000000000090 <try_wait_for_completion>:
90: 41 54 push %r12
92: 55 push %rbp
93: 31 ed xor %ebp,%ebp
95: 53 push %rbx
96: 8b 07 mov (%rdi),%eax
98: 85 c0 test %eax,%eax
9a: 75 07 jne a3 <try_wait_for_completion+0x13>
9c: 89 e8 mov %ebp,%eax
9e: 5b pop %rbx
9f: 5d pop %rbp
a0: 41 5c pop %r12
a2: c3 retq
a3: 4c 8d 67 08 lea 0x8(%rdi),%r12
a7: 48 89 fb mov %rdi,%rbx
aa: 4c 89 e7 mov %r12,%rdi
ad: e8 00 00 00 00 callq b2 <try_wait_for_completion+0x22>
ae: R_X86_64_PC32 _raw_spin_lock_irqsave-0x4
b2: 8b 13 mov (%rbx),%edx
b4: 85 d2 test %edx,%edx
b6: 74 0f je c7 <try_wait_for_completion+0x37>
b8: 83 fa ff cmp $0xffffffff,%edx
bb: bd 01 00 00 00 mov $0x1,%ebp
c0: 74 05 je c7 <try_wait_for_completion+0x37>
c2: 83 ea 01 sub $0x1,%edx
c5: 89 13 mov %edx,(%rbx)
c7: 48 89 c6 mov %rax,%rsi
ca: 4c 89 e7 mov %r12,%rdi
cd: e8 00 00 00 00 callq d2 <try_wait_for_completion+0x42>
ce: R_X86_64_PC32 _raw_spin_unlock_irqrestore-0x4
d2: 89 e8 mov %ebp,%eax
d4: 5b pop %rbx
d5: 5d pop %rbp
d6: 41 5c pop %r12
d8: c3 retq
d9: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
Note how it keeps the return value in eax and doesn't spill to the
stack. And I would expect this to be true for most architectures that
have register based calling conventions, its an otherwise fairly trivial
function.
I'll take the patch though, but I'll remove that last bit from the
Changelog.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH]sched: completion: use bool in try_wait_for_completion
2018-02-21 13:28 ` Peter Zijlstra
@ 2018-02-21 13:49 ` gaurav jindal
0 siblings, 0 replies; 7+ messages in thread
From: gaurav jindal @ 2018-02-21 13:49 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: mingo, linux-kernel
On Wed, Feb 21, 2018 at 02:28:54PM +0100, Peter Zijlstra wrote:
> On Wed, Feb 21, 2018 at 06:24:07PM +0530, gaurav jindal wrote:
> > Variable ret in try_wait_for_completion can have only true/false values. Since
> > the return type of the function is also bool, variable ret should have data
> > type as bool in place of int.
>
> Fair enough.
>
> > Moreover, the size of bool in many platforms is 1 byte whereas size of int is
> > 4 bytes(though architecture dependent). Modifying the data type reduces the
> > size consumed for the stack.
>
> Absolutely 0 difference in generated assembly here on x86_64-defconfig
> gcc Debian 7.2.0-20.
>
> $ objdump -dr defconfig-build/kernel/sched/completion.o | awk '/^$/ {p=0} /<try_wait_for_completion>:$/ {p=1} {if (p) print $0}'
>
> 0000000000000090 <try_wait_for_completion>:
> 90: 41 54 push %r12
> 92: 55 push %rbp
> 93: 31 ed xor %ebp,%ebp
> 95: 53 push %rbx
> 96: 8b 07 mov (%rdi),%eax
> 98: 85 c0 test %eax,%eax
> 9a: 75 07 jne a3 <try_wait_for_completion+0x13>
> 9c: 89 e8 mov %ebp,%eax
> 9e: 5b pop %rbx
> 9f: 5d pop %rbp
> a0: 41 5c pop %r12
> a2: c3 retq
> a3: 4c 8d 67 08 lea 0x8(%rdi),%r12
> a7: 48 89 fb mov %rdi,%rbx
> aa: 4c 89 e7 mov %r12,%rdi
> ad: e8 00 00 00 00 callq b2 <try_wait_for_completion+0x22>
> ae: R_X86_64_PC32 _raw_spin_lock_irqsave-0x4
> b2: 8b 13 mov (%rbx),%edx
> b4: 85 d2 test %edx,%edx
> b6: 74 0f je c7 <try_wait_for_completion+0x37>
> b8: 83 fa ff cmp $0xffffffff,%edx
> bb: bd 01 00 00 00 mov $0x1,%ebp
> c0: 74 05 je c7 <try_wait_for_completion+0x37>
> c2: 83 ea 01 sub $0x1,%edx
> c5: 89 13 mov %edx,(%rbx)
> c7: 48 89 c6 mov %rax,%rsi
> ca: 4c 89 e7 mov %r12,%rdi
> cd: e8 00 00 00 00 callq d2 <try_wait_for_completion+0x42>
> ce: R_X86_64_PC32 _raw_spin_unlock_irqrestore-0x4
> d2: 89 e8 mov %ebp,%eax
> d4: 5b pop %rbx
> d5: 5d pop %rbp
> d6: 41 5c pop %r12
> d8: c3 retq
> d9: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
>
> Note how it keeps the return value in eax and doesn't spill to the
> stack. And I would expect this to be true for most architectures that
> have register based calling conventions, its an otherwise fairly trivial
> function.
>
I completely agree. I got carried away with sizeof(). Missed the case of using
the local registers.
Thanks a lot for guiding me again.
> I'll take the patch though, but I'll remove that last bit from the
> Changelog.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:sched/core] sched/completions: Use bool in try_wait_for_completion()
2018-02-21 12:54 [PATCH]sched: completion: use bool in try_wait_for_completion gaurav jindal
2018-02-21 13:28 ` Peter Zijlstra
@ 2018-03-09 9:06 ` tip-bot for gaurav jindal
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for gaurav jindal @ 2018-03-09 9:06 UTC (permalink / raw)
To: linux-tip-commits
Cc: tglx, linux-kernel, hpa, peterz, gauravjindal1104, mingo, torvalds
Commit-ID: d17067e4487adc53bedb43681b3cb5a1714ff6ca
Gitweb: https://git.kernel.org/tip/d17067e4487adc53bedb43681b3cb5a1714ff6ca
Author: gaurav jindal <gauravjindal1104@gmail.com>
AuthorDate: Wed, 21 Feb 2018 18:24:07 +0530
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 9 Mar 2018 08:00:18 +0100
sched/completions: Use bool in try_wait_for_completion()
Since the return type of the function is bool, the internal
'ret' variable should be bool too.
Signed-off-by: Gaurav Jindal<gauravjindal1104@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180221125407.GA14292@gmail.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/completion.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 5d2d56b0817a..e426b0cb9ac6 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -280,7 +280,7 @@ EXPORT_SYMBOL(wait_for_completion_killable_timeout);
bool try_wait_for_completion(struct completion *x)
{
unsigned long flags;
- int ret = 1;
+ bool ret = true;
/*
* Since x->done will need to be locked only
@@ -289,11 +289,11 @@ bool try_wait_for_completion(struct completion *x)
* return early in the blocking case.
*/
if (!READ_ONCE(x->done))
- return 0;
+ return false;
spin_lock_irqsave(&x->wait.lock, flags);
if (!x->done)
- ret = 0;
+ ret = false;
else if (x->done != UINT_MAX)
x->done--;
spin_unlock_irqrestore(&x->wait.lock, flags);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH]sched: completion: use bool in try_wait_for_completion
@ 2018-02-14 10:30 gaurav jindal
2018-02-19 10:44 ` Peter Zijlstra
0 siblings, 1 reply; 7+ messages in thread
From: gaurav jindal @ 2018-02-14 10:30 UTC (permalink / raw)
To: peterz, mingo; +Cc: linux-kernel
Use bool in place of int in the function try_wait_for_completion.
Signed-off-by: Gaurav Jindal<gauravjindal1104@gmail.com>
---
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 0926aef..3e15e8d 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -283,7 +283,7 @@ int __sched wait_for_completion_killable(struct completion *x)
bool try_wait_for_completion(struct completion *x)
{
unsigned long flags;
- int ret = 1;
+ bool ret = true;
/*
* Since x->done will need to be locked only
@@ -292,11 +292,11 @@ bool try_wait_for_completion(struct completion *x)
* return early in the blocking case.
*/
if (!READ_ONCE(x->done))
- return 0;
+ return false;
spin_lock_irqsave(&x->wait.lock, flags);
if (!x->done)
- ret = 0;
+ ret = false;
else if (x->done != UINT_MAX)
x->done--;
spin_unlock_irqrestore(&x->wait.lock, flags);
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH]sched: completion: use bool in try_wait_for_completion
@ 2018-01-24 9:57 gaurav jindal
0 siblings, 0 replies; 7+ messages in thread
From: gaurav jindal @ 2018-01-24 9:57 UTC (permalink / raw)
To: peterz, mingo; +Cc: linux-kernel
Use bool in place of int in the function try_wait_for_completion.
Signed-off-by: Gaurav Jindal<gauravjindal1104@gmail.com>
---
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 0926aef..3e15e8d 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -283,7 +283,7 @@ int __sched wait_for_completion_killable(struct completion *x)
bool try_wait_for_completion(struct completion *x)
{
unsigned long flags;
- int ret = 1;
+ bool ret = true;
/*
* Since x->done will need to be locked only
@@ -292,11 +292,11 @@ bool try_wait_for_completion(struct completion *x)
* return early in the blocking case.
*/
if (!READ_ONCE(x->done))
- return 0;
+ return false;
spin_lock_irqsave(&x->wait.lock, flags);
if (!x->done)
- ret = 0;
+ ret = false;
else if (x->done != UINT_MAX)
x->done--;
spin_unlock_irqrestore(&x->wait.lock, flags);
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-09 9:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-21 12:54 [PATCH]sched: completion: use bool in try_wait_for_completion gaurav jindal
2018-02-21 13:28 ` Peter Zijlstra
2018-02-21 13:49 ` gaurav jindal
2018-03-09 9:06 ` [tip:sched/core] sched/completions: Use bool in try_wait_for_completion() tip-bot for gaurav jindal
-- strict thread matches above, loose matches on Subject: below --
2018-02-14 10:30 [PATCH]sched: completion: use bool in try_wait_for_completion gaurav jindal
2018-02-19 10:44 ` Peter Zijlstra
2018-01-24 9:57 gaurav jindal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).