Re: [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code
Le Wed, 26 Sep 2007 22:47:54 +0200,
roel <[EMAIL PROTECTED]> a écrit :
> > + if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
>
> if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))
Yeah, right, good catch.
> > + group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
> > + stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
>
> Isn't this just confusing? why not
>
> if (tg_stats) {
> group_exit_code = tsk->signal->group_exit_code;
> stats->ac_exitcode = group_exit_code;
Because in this case if group_exit_code is null, we want
tsk->exit_code, not 0.
>
> } else {
> group_exit_code = 0;
> stats->ac_exitcode = tsk->exit_code;
> }
Andrew is not interested at the moment in this series (that replaces
all my previous patches on taskstats, for info), but thank you for the
review.
--
Guillaume
-
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 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code
Guillaume Chazarain wrote:
[...]
> @@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct
> task_struct *tsk)
> void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
> bool tg_stats)
> {
> + int group_exit_code;
> +
> fill_wall_times(stats, tsk);
>
> - if (thread_group_leader(tsk)) {
> - stats->ac_exitcode = tsk->exit_code;
> - if (tsk->flags & PF_FORKNOEXEC)
> - stats->ac_flag |= AFORK;
> - }
> + if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))
> + stats->ac_flag |= AFORK;
> +
> + group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
> + stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
Isn't this just confusing? why not
if (tg_stats) {
group_exit_code = tsk->signal->group_exit_code;
stats->ac_exitcode = group_exit_code;
} else {
group_exit_code = 0;
stats->ac_exitcode = tsk->exit_code;
}
-
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 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code
Threads also have an exit code on their own, so report it in
TASKSTATS_CMD_ATTR_PID.
For TASKSTATS_CMD_ATTR_TGID, instead of relying only on the exit code of the
leader, we use task->signal->group_exit_code if not null as suggested by
Oleg Nesterov.
Also, document that as of this patch, fill_threadgroup_stats() must be called
after add_tsk_stats() as it may overwrite some stats.
Signed-off-by: Guillaume Chazarain <[EMAIL PROTECTED]>
Cc: Balbir Singh <[EMAIL PROTECTED]>
Cc: Jay Lan <[EMAIL PROTECTED]>
Cc: Jonathan Lim <[EMAIL PROTECTED]>
Cc: Oleg Nesterov <[EMAIL PROTECTED]>
---
kernel/taskstats.c |2 ++
kernel/tsacct.c| 12 +++-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 59fe1d8..938c0b0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -208,6 +208,8 @@ static void add_tsk_stats(struct taskstats *stats, struct
task_struct *task)
*
* Will be called on the thread group leader if requesting stats for the whole
* thread group.
+ * fill_threadgroup_stats() may overwrite stats from add_tsk_stats(), so it
+ * must be called after add_tsk_stats().
*/
static void fill_threadgroup_stats(struct taskstats *stats,
struct task_struct *task, bool tg_stats)
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 8c2f470..e83f53b 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct
task_struct *tsk)
void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
bool tg_stats)
{
+ int group_exit_code;
+
fill_wall_times(stats, tsk);
- if (thread_group_leader(tsk)) {
- stats->ac_exitcode = tsk->exit_code;
- if (tsk->flags & PF_FORKNOEXEC)
- stats->ac_flag |= AFORK;
- }
+ if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
+ stats->ac_flag |= AFORK;
+
+ group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
+ stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
stats->ac_nice = task_nice(tsk);
stats->ac_sched = tsk->policy;
-
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 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code
Threads also have an exit code on their own, so report it in
TASKSTATS_CMD_ATTR_PID.
For TASKSTATS_CMD_ATTR_TGID, instead of relying only on the exit code of the
leader, we use task->signal->group_exit_code if not null as suggested by
Oleg Nesterov.
Also, document that as of this patch, fill_threadgroup_stats() must be called
after add_tsk_stats() as it may overwrite some stats.
Signed-off-by: Guillaume Chazarain <[EMAIL PROTECTED]>
Cc: Balbir Singh <[EMAIL PROTECTED]>
Cc: Jay Lan <[EMAIL PROTECTED]>
Cc: Jonathan Lim <[EMAIL PROTECTED]>
Cc: Oleg Nesterov <[EMAIL PROTECTED]>
---
kernel/taskstats.c |2 ++
kernel/tsacct.c| 12 +++-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 59fe1d8..938c0b0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -208,6 +208,8 @@ static void add_tsk_stats(struct taskstats *stats, struct
task_struct *task)
*
* Will be called on the thread group leader if requesting stats for the whole
* thread group.
+ * fill_threadgroup_stats() may overwrite stats from add_tsk_stats(), so it
+ * must be called after add_tsk_stats().
*/
static void fill_threadgroup_stats(struct taskstats *stats,
struct task_struct *task, bool tg_stats)
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 8c2f470..e83f53b 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct
task_struct *tsk)
void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
bool tg_stats)
{
+ int group_exit_code;
+
fill_wall_times(stats, tsk);
- if (thread_group_leader(tsk)) {
- stats->ac_exitcode = tsk->exit_code;
- if (tsk->flags & PF_FORKNOEXEC)
- stats->ac_flag |= AFORK;
- }
+ if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
+ stats->ac_flag |= AFORK;
+
+ group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
+ stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
stats->ac_nice = task_nice(tsk);
stats->ac_sched = tsk->policy;
-
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/
