taskstats.ac_exitcode is assigned to task_struct.exit_code in bacct_add_tsk()
through the following kernel function calls:
do_exit()
taskstats_exit_send()
fill_pid()
bacct_add_tsk()
The problem is that in do_exit(), task_struct.exit_code is set to 'code'
only after taskstats_exit_send() has been called. So we need to send 'code'
through to bacct_add_tsk().
Diff'd against: linux/kernel/git/stable/linux-2.6.22.y.git
Signed-off-by: Jonathan Lim <[email protected]>
--- a/kernel/exit.c 2007-08-20 16:37:55.000000000 -0700
+++ b/kernel/exit.c 2007-08-20 16:37:55.000000000 -0700
@@ -941,7 +941,7 @@ fastcall NORET_TYPE void do_exit(long co
if (unlikely(tsk->audit_context))
audit_free(tsk);
- taskstats_exit(tsk, group_dead);
+ taskstats_exit(code, tsk, group_dead);
exit_mm(tsk);
--- a/kernel/taskstats.c 2007-08-20 16:37:55.000000000 -0700
+++ b/kernel/taskstats.c 2007-08-20 16:37:55.000000000 -0700
@@ -168,7 +168,7 @@ static void send_cpu_listeners(struct sk
up_write(&listeners->sem);
}
-static int fill_pid(pid_t pid, struct task_struct *tsk,
+static int fill_pid(long exitcode, pid_t pid, struct task_struct *tsk,
struct taskstats *stats)
{
int rc = 0;
@@ -196,7 +196,7 @@ static int fill_pid(pid_t pid, struct ta
/* fill in basic acct fields */
stats->version = TASKSTATS_VERSION;
- bacct_add_tsk(stats, tsk);
+ bacct_add_tsk(exitcode, stats, tsk);
/* fill in extended acct fields */
xacct_add_tsk(stats, tsk);
@@ -452,7 +452,7 @@ ret:
}
/* Send pid data out on exit */
-void taskstats_exit(struct task_struct *tsk, int group_dead)
+void taskstats_exit(long exitcode, struct task_struct *tsk, int group_dead)
{
int rc;
struct listener_list *listeners;
@@ -490,7 +490,7 @@ void taskstats_exit(struct task_struct *
if (!stats)
goto err;
- rc = fill_pid(tsk->pid, tsk, stats);
+ rc = fill_pid(exitcode, tsk->pid, tsk, stats);
if (rc < 0)
goto err;
--- a/kernel/tsacct.c 2007-08-20 16:37:55.000000000 -0700
+++ b/kernel/tsacct.c 2007-08-20 16:37:55.000000000 -0700
@@ -25,7 +25,7 @@
/*
* fill in basic accounting fields
*/
-void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+void bacct_add_tsk(long exitcode, struct taskstats *stats, struct task_struct *tsk)
{
struct timespec uptime, ts;
s64 ac_etime;
@@ -41,7 +41,7 @@ void bacct_add_tsk(struct taskstats *sta
stats->ac_etime = ac_etime;
stats->ac_btime = xtime.tv_sec - ts.tv_sec;
if (thread_group_leader(tsk)) {
- stats->ac_exitcode = tsk->exit_code;
+ stats->ac_exitcode = exitcode;
if (tsk->flags & PF_FORKNOEXEC)
stats->ac_flag |= AFORK;
}
--- a/include/linux/taskstats_kern.h 2007-08-20 18:13:11.000000000 -0700
+++ b/include/linux/taskstats_kern.h 2007-08-20 18:16:12.000000000 -0700
@@ -26,10 +26,10 @@ static inline void taskstats_tgid_free(s
kmem_cache_free(taskstats_cache, sig->stats);
}
-extern void taskstats_exit(struct task_struct *, int group_dead);
+extern void taskstats_exit(long exitcode, struct task_struct *, int group_dead);
extern void taskstats_init_early(void);
#else
-static inline void taskstats_exit(struct task_struct *tsk, int group_dead)
+static inline void taskstats_exit(long exitcode, struct task_struct *tsk, int group_dead)
{}
static inline void taskstats_tgid_init(struct signal_struct *sig)
{}
--- a/include/linux/tsacct_kern.h 2007-08-20 18:13:11.000000000 -0700
+++ b/include/linux/tsacct_kern.h 2007-08-20 18:17:55.000000000 -0700
@@ -10,9 +10,9 @@
#include <linux/taskstats.h>
#ifdef CONFIG_TASKSTATS
-extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
+extern void bacct_add_tsk(long exitcode, struct taskstats *stats, struct task_struct *tsk);
#else
-static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+static inline void bacct_add_tsk(long exitcode, struct taskstats *stats, struct task_struct *tsk)
{}
#endif /* CONFIG_TASKSTATS */
Jonathan Lim wrote:
> taskstats.ac_exitcode is assigned to task_struct.exit_code in bacct_add_tsk()
> through the following kernel function calls:
>
> do_exit()
> taskstats_exit_send()
> fill_pid()
> bacct_add_tsk()
>
> The problem is that in do_exit(), task_struct.exit_code is set to 'code'
> only after taskstats_exit_send() has been called. So we need to send 'code'
> through to bacct_add_tsk().
>
Hi, Jonathan,
The patches look like a step in the right direction, I would suggest an alternate
implementation
Why can't we assign tsk->exit_code to code earlier? Can we not move up the assignment
to before taskstats_exit()? Wouldn't that be much simpler?
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
On Mon Aug 20 20:44:13 2007, [email protected] wrote:
>
> Jonathan Lim wrote:
> > taskstats.ac_exitcode is assigned to task_struct.exit_code in
> > bacct_add_tsk() through the following kernel function calls:
> >
> > do_exit()
> > taskstats_exit_send()
> > fill_pid()
> > bacct_add_tsk()
> >
> > The problem is that in do_exit(), task_struct.exit_code is set to 'code'
> > only after taskstats_exit_send() has been called. So we need to send
> > 'code' through to bacct_add_tsk().
>
> Hi, Jonathan,
>
> The patches look like a step in the right direction, I would suggest an
> alternate implementation
>
> Why can't we assign tsk->exit_code to code earlier? Can we not move up the
> assignment to before taskstats_exit()? Wouldn't that be much simpler?
Hi Balbir,
That was what I wanted to do at first, but there was some concern over whether
it would affect any of the intervening function calls. If there's no
particular reason why the tsk->exit_code assignment is placed where it's at
now, then yes, I would much rather move it to before taskstats_exit().
I looked at the following functions involving tsk:
exit_mm
mm_release
deactivate_mm
exit_sem
__exit_files
__exit_fs
cpuset_exit
exit_keys
and don't see anything that setting exit_code would affect. What do you think?
Jonathan
Jonathan Lim wrote:
> On Mon Aug 20 20:44:13 2007, [email protected] wrote:
>> Jonathan Lim wrote:
>>> taskstats.ac_exitcode is assigned to task_struct.exit_code in
>>> bacct_add_tsk() through the following kernel function calls:
>>>
>>> do_exit()
>>> taskstats_exit_send()
>>> fill_pid()
>>> bacct_add_tsk()
>>>
>>> The problem is that in do_exit(), task_struct.exit_code is set to 'code'
>>> only after taskstats_exit_send() has been called. So we need to send
>>> 'code' through to bacct_add_tsk().
>> Hi, Jonathan,
>>
>> The patches look like a step in the right direction, I would suggest an
>> alternate implementation
>>
>> Why can't we assign tsk->exit_code to code earlier? Can we not move up the
>> assignment to before taskstats_exit()? Wouldn't that be much simpler?
>
> Hi Balbir,
>
> That was what I wanted to do at first, but there was some concern over whether
> it would affect any of the intervening function calls. If there's no
> particular reason why the tsk->exit_code assignment is placed where it's at
> now, then yes, I would much rather move it to before taskstats_exit().
>
> I looked at the following functions involving tsk:
>
> exit_mm
> mm_release
> deactivate_mm
> exit_sem
> __exit_files
> __exit_fs
> cpuset_exit
> exit_keys
>
> and don't see anything that setting exit_code would affect. What do you think?
>
I think your search and analysis leads me to believe that, it might be the
correct thing to do. I would suggest we patch it that way and run a functional
test like LTP to ensure we did not break anything.
What do you think?
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
On Tue Aug 21 11:27:07 2007, [email protected] wrote:
>
> > Hi Balbir,
> >
> > That was what I wanted to do at first, but there was some concern over
> > whether it would affect any of the intervening function calls. If there's
> > no particular reason why the tsk->exit_code assignment is placed where it's
> > at now, then yes, I would much rather move it to before taskstats_exit().
> >
> > I looked at the following functions involving tsk:
> >
> > exit_mm
> > mm_release
> > deactivate_mm
> > exit_sem
> > __exit_files
> > __exit_fs
> > cpuset_exit
> > exit_keys
> >
> > and don't see anything that setting exit_code would affect. What do you
> > think?
>
> I think your search and analysis leads me to believe that, it might be the
> correct thing to do. I would suggest we patch it that way and run a
> functional test like LTP to ensure we did not break anything.
>
> What do you think?
Okay, I'll post a revised patch next. Thanks.
Jonathan