Re: [PATCH] Fix tsk->exit_state usage (resend)
Hi Eugene, This already got merged into -mm, but ... On Sun, 19 Aug 2007, Eugene Teo wrote: > > tsk->exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test > is the same as tsk->exit_state & (EXIT_ZOMBIE | EXIT_DEAD), so just testing > tsk->exit_state is sufficient. ... IMHO this change harms the readability of the code. > +++ b/fs/proc/array.c > @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct > task_struct *tsk) > TASK_UNINTERRUPTIBLE | > TASK_STOPPED | > TASK_TRACED)) | > - (tsk->exit_state & (EXIT_ZOMBIE | > - EXIT_DEAD)); > +tsk->exit_state; Here, for example, the code is /purposefully/ enumerating all the task states, probably it makes sense to explicitly enumerate the exit states as well? > +++ b/kernel/fork.c > @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); > > void __put_task_struct(struct task_struct *tsk) > { > - WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE))); > + WARN_ON(!tsk->exit_state); > +++ b/kernel/sched.c > @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct > task_struct *p) > struct rq *rq = cpu_rq(dead_cpu); > > /* Must be exiting, otherwise would be on tasklist. */ > - BUG_ON(p->exit_state != EXIT_ZOMBIE && p->exit_state != EXIT_DEAD); > + BUG_ON(!p->exit_state); Regarding above two changes -- agreed, we want to catch /any/ exiting task state, so (!p->exit_state) is /correct/, but still, enumerating those explicitly helps readability. And although it's unlikely, in the future, we may have an exit_state value for which we may _not_ want to complain (WARN or BUG) in this code. So I'd still vote to keep the code explicit like it was ... Satyam - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Fix tsk-exit_state usage (resend)
Hi Eugene, This already got merged into -mm, but ... On Sun, 19 Aug 2007, Eugene Teo wrote: tsk-exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk-exit_state (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk-exit_state is sufficient. ... IMHO this change harms the readability of the code. +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk-exit_state (EXIT_ZOMBIE | - EXIT_DEAD)); +tsk-exit_state; Here, for example, the code is /purposefully/ enumerating all the task states, probably it makes sense to explicitly enumerate the exit states as well? +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk-exit_state (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk-exit_state); +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p-exit_state != EXIT_ZOMBIE p-exit_state != EXIT_DEAD); + BUG_ON(!p-exit_state); Regarding above two changes -- agreed, we want to catch /any/ exiting task state, so (!p-exit_state) is /correct/, but still, enumerating those explicitly helps readability. And although it's unlikely, in the future, we may have an exit_state value for which we may _not_ want to complain (WARN or BUG) in this code. So I'd still vote to keep the code explicit like it was ... Satyam - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk->exit_state usage (resend)
tsk->exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk->exit_state & (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk->exit_state is sufficient. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk->exit_state & (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk->exit_state; const char **p = _state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk->exit_state); WARN_ON(atomic_read(>usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p->exit_state != EXIT_ZOMBIE && p->exit_state != EXIT_DEAD); + BUG_ON(!p->exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p->state == TASK_DEAD); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk-exit_state usage (resend)
tsk-exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk-exit_state (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk-exit_state is sufficient. Signed-off-by: Eugene Teo [EMAIL PROTECTED] --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk-exit_state (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk-exit_state; const char **p = task_state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk-exit_state (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk-exit_state); WARN_ON(atomic_read(tsk-usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p-exit_state != EXIT_ZOMBIE p-exit_state != EXIT_DEAD); + BUG_ON(!p-exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p-state == TASK_DEAD); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk->exit_state usage
tsk->exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk->exit_state & (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk->exit_state is sufficient. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk->exit_state & (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk->exit_state; const char **p = _state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk->exit_state); WARN_ON(atomic_read(>usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p->exit_state != EXIT_ZOMBIE && p->exit_state != EXIT_DEAD); + BUG_ON(!p->exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p->state == TASK_DEAD); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk-exit_state usage
tsk-exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk-exit_state (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk-exit_state is sufficient. Signed-off-by: Eugene Teo [EMAIL PROTECTED] --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk-exit_state (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk-exit_state; const char **p = task_state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk-exit_state (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk-exit_state); WARN_ON(atomic_read(tsk-usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p-exit_state != EXIT_ZOMBIE p-exit_state != EXIT_DEAD); + BUG_ON(!p-exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p-state == TASK_DEAD); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/