2008-10-31 08:53:18

by Zhao Lei

Subject: [PATCH] fork.c: cleanup for copy_sighand()

Check CLONE_SIGHAND only is enough, because combination of CLONE_THREAD and
CLONE_SIGHAND is already done in copy_process().

Impact: cleanup, no functionality changed

Signed-off-by: Zhao Lei <[email protected]>
---
kernel/fork.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index f608356..36a0dac 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -741,7 +741,7 @@ static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
{
struct sighand_struct *sig;

- if (clone_flags & (CLONE_SIGHAND | CLONE_THREAD)) {
+ if (clone_flags & CLONE_SIGHAND) {
atomic_inc(&current->sighand->count);
return 0;
}
--
1.5.5.3

2008-11-04 23:59:04

by Andrew Morton

Subject: Re: [PATCH] fork.c: cleanup for copy_sighand()

On Fri, 31 Oct 2008 16:51:19 +0800
Zhaolei <[email protected]> wrote:

> Check CLONE_SIGHAND only is enough, because combination of CLONE_THREAD and
> CLONE_SIGHAND is already done in copy_process().
>
> Impact: cleanup, no functionality changed
>
> Signed-off-by: Zhao Lei <[email protected]>
> ---
> kernel/fork.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index f608356..36a0dac 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -741,7 +741,7 @@ static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk)
> {
> struct sighand_struct *sig;
>
> - if (clone_flags & (CLONE_SIGHAND | CLONE_THREAD)) {
> + if (clone_flags & CLONE_SIGHAND) {
> atomic_inc(&current->sighand->count);
> return 0;
> }

OK, it appears to be correct and might yield a tiny speedup on some
architectures.

But whether this change is desirable from a clarity and maintainability
point of view is unclear to me. Let's add the cc's..