Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   ebiederm@xmission.com (Eric W. Biederman)
To:     <Peter.Enderborg@sony.com>
Cc:     <mathieu.desnoyers@efficios.com>, <rostedt@goodmis.org>,
        <linux-kernel@vger.kernel.org>, <mingo@redhat.com>,
        <akpm@linux-foundation.org>, <peterz@infradead.org>,
        <ast@kernel.org>, <christian.brauner@ubuntu.com>,
        <dave@stgolabs.net>, <walken@google.com>, <jannh@google.com>,
        <christophe.leroy@c-s.fr>, <minchan@kernel.org>
References: <20210430142223.25500-1-peter.enderborg@sony.com>
        <20210430142223.25500-2-peter.enderborg@sony.com>
        <m14kfnzmsp.fsf@fess.ebiederm.org>
        <4bb24db4-f720-f5e7-9054-36bdeaee1d79@sony.com>
        <20210501091104.418765bd@oasis.local.home>
        <1214833117.22933.1620049830326.JavaMail.zimbra@efficios.com>
        <769a00d2-a76d-62d2-aa56-5a107dfdc53d@sony.com>
Date:   Mon, 03 May 2021 11:30:55 -0500
In-Reply-To: <769a00d2-a76d-62d2-aa56-5a107dfdc53d@sony.com> (Peter
        Enderborg's message of "Mon, 3 May 2021 14:48:33 +0000")
Message-ID: <m1sg33lqyo.fsf@fess.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Subject: Re: [PATCH 1/2] tracing: Add a trace for task_exit
Precedence: bulk

<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