pid: remove redundant condition
PATCH 1/1 pid: remove redundant condition
From: Liu Ping Fan <[email protected]>
--- kernel/pid.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
kernel/pid.c +1 -2
@@ -248,8 +248,7 @@ void put_pid(struct pid *pid)return ; ns = pid->numbers[pid->level].ns; -if ((atomic_read (&pid->count) ==1 ) || -atomic_dec_and_test (&pid->count)) { + if (atomic_dec_and_test(&pid->count) {kmem_cache_free (ns->pid_cachep, pid);put_pid_ns (ns); }
Signature · 1 line
1.7.4.4
Quoted 12 lines · Liu Ping Fan
From: Liu Ping Fan <[email protected]>
Signed-off-by Liu Ping Fan <[email protected]>---
kernel/pid.c | 3 +--1 files changed, 1 insertions(+), 2 deletions(-)kernel/pid.c +0
-0
kernel/pid.c void put_pid(struct pid *pid) · line 254
Quoted 9 lines
kernel/pid.c +1
-2@@ -248,8 +248,7 @@ void put_pid(struct pid *pid) return; ns = pid->numbers[pid->level].ns; - if ((atomic_read(&pid->count) == 1) || - atomic_dec_and_test(&pid->count)) { + if (atomic_dec_and_test(&pid->count) { kmem_cache_free(ns->pid_cachep, pid); put_pid_ns(ns); }
No, this isn't necessarily redundant.
The atomic_dec_and_test() instruction can be very expensive. So if count was 1 before, we're the last user, and we can free it without the expensive atomic_dec_and_test().
However, if we are *not* the last user, then we can race with another user decrementing the count, so now we need to use the expensive version.
Now, whether that special case is really worth it or not, I don't know. But it's not redundant.
Linus
Quoted 25 lines · Linus Torvalds
Quoted 12 lines · Liu Ping Fan
From: Liu Ping Fan <[email protected]>
Signed-off-by Liu Ping Fan <[email protected]>---
kernel/pid.c | 3 +--1 files changed, 1 insertions(+), 2 deletions(-)kernel/pid.c +0
-0kernel/pid.c void put_pid(struct pid *pid) · line 254
Quoted 9 lines
kernel/pid.c +1
-2@@ -248,8 +248,7 @@ void put_pid(struct pid *pid) return; ns = pid->numbers[pid->level].ns; - if ((atomic_read(&pid->count) == 1) || - atomic_dec_and_test(&pid->count)) { + if (atomic_dec_and_test(&pid->count) { kmem_cache_free(ns->pid_cachep, pid); put_pid_ns(ns); }No, this isn't necessarily redundant.
Quoted 1 line
The
atomic_dec_and_test()instruction can be very expensive. So if
Oh, got it, thanks.
Regards,
pingfan
Quoted 11 lines
count was 1 before, we're the last user, and we can free it without the expensive
atomic_dec_and_test().However, if we are *not* the last user, then we can race with another user decrementing the count, so now we need to use the expensive version.
Now, whether that special case is really worth it or not, I don't know. But it's not redundant.
Linus