Re: [PATCH 1/2] tracing: Add a trace for task_exit

[ebiederm@xmission.com (Eric W. Biederman)]

<Peter.Enderborg@sony.com> writes:

> On 5/3/21 3:50 PM, Mathieu Desnoyers wrote:
>> ----- On May 1, 2021, at 9:11 AM, rostedt rostedt@goodmis.org wrote:
>>
>>> On Sat, 1 May 2021 09:29:41 +0000
>>> <Peter.Enderborg@sony.com> wrote:
>>>
>>>> On 4/30/21 7:48 PM, Eric W. Biederman wrote:
>>>>> Peter Enderborg <peter.enderborg@sony.com> writes:
>>>>>  
>>>>>> This is the peer functions to task_rename and task_newtask.
>>>>>> With this we get hole "life-cycle" of task and can easily
>>>>>> see short livied task and their exit status.
>>>>> This patch is incorrect.  The location you are dealing with is not part
>>>>> of task exit.  The location you have instrumented is part of reaping a
>>>>> task which can come arbitrarily long after the task exits.
>>>> That is what it aiming. When using this as tool for userspace you
>>>> would like to know when the task is done. When it no longer
>>>> holds any thing that might have any impact. If you think the
>>>> exit imply something more specific I can change the name.
>>>>
>>>> I thought exit was a good name, it is in in exit.c.
>>>>
>>>> Will the name task_done, task_finished or task_reaped work for you?
>>> I think "task_reaped" is probably the best name, and the most
>>> descriptive of what happened.
>> What would it provide that is not already available through the "sched_process_free"
>> tracepoint in delayed_put_task_struct ?
>
> For task_exit (or task_reaped)
>
>         field:pid_t pid;        offset:8;       size:4; signed:1;
>         field:short oom_score_adj;      offset:12;      size:2; signed:1;
>         field:int exit_signal;  offset:16;      size:4; signed:1;
>         field:int exit_code;    offset:20;      size:4; signed:1;
>         field:int exit_state;   offset:24;      size:4; signed:1;
>         field:__data_loc char[] comm;   offset:28;      size:4; signed:1;
>
> sched_process_free
>         field:char comm[16];    offset:8;       size:16;        signed:1;
>         field:pid_t pid;        offset:24;      size:4; signed:1;
>         field:int prio; offset:28;      size:4; signed:1;
>
> So information about oom_score_adj, and it's exit parameters.


For the record returning oom_score_adj that late is not appropriate for
any kernel/user API.  It is perfectly valid for the kernel to optimize
out anything that wait(2) does not return.

If you want oom_score_adj you probably need to sample it in
sched_process_exit.

I periodically move things from the point a process is reaped to the
point where a task stops running, for both correctness and for simpler
maintenance.  When threads were added a bunch of cleanup was added
to the wrong place.  I certainly would not hesitate to mess with
oom_score_adj if changing something would make the code simpler.

With both sched_process_free and sched_process_exit it looks like we
already have tracepoints everywhere they could be needed.
task exit.

Eric