2024-02-23 15:27:50

by Wen Yang

[permalink] [raw]
Subject: [PATCH v2] exit: add exit_code to trace_sched_process_exit and move it earlier in do_exit()

From: Wen Yang <[email protected]>

Currently coredump_task_exit() takes some time to wait for the generation
of the dump file. But if the user-space wants to receive a notification
as soon as possible it maybe inconvenient.

Add exit_code to the TP trace_sched_process_exit() and move it earlier in
do_exit(). This way a user-space monitor could detect the exits and
potentially make some preparations in advance.

Suggested-by: Steven Rostedt <[email protected]>
Signed-off-by: Wen Yang <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mel Gorman <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected]
---
include/trace/events/sched.h | 28 +++++++++++++++++++++++++---
kernel/exit.c | 2 +-
2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index dbb01b4b7451..c2e8655fd453 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -330,9 +330,31 @@ DEFINE_EVENT(sched_process_template, sched_process_free,
/*
* Tracepoint for a task exiting:
*/
-DEFINE_EVENT(sched_process_template, sched_process_exit,
- TP_PROTO(struct task_struct *p),
- TP_ARGS(p));
+TRACE_EVENT(sched_process_exit,
+
+ TP_PROTO(struct task_struct *task, long code),
+
+ TP_ARGS(task, code),
+
+ TP_STRUCT__entry(
+ __array( char, comm, TASK_COMM_LEN )
+ __field( pid_t, pid )
+ __field( int, prio )
+ __field( long, code )
+ ),
+
+ TP_fast_assign(
+ memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
+ __entry->pid = task->pid;
+ __entry->prio = task->prio;
+ __entry->code = code;
+ ),
+
+ TP_printk("comm=%s pid=%d prio=%d exit_code=0x%lx",
+ __entry->comm, __entry->pid, __entry->prio,
+ __entry->code)
+);
+

/*
* Tracepoint for waiting on task to unschedule:
diff --git a/kernel/exit.c b/kernel/exit.c
index 493647fd7c07..48b6ed7f7760 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -826,6 +826,7 @@ void __noreturn do_exit(long code)

WARN_ON(tsk->plug);

+ trace_sched_process_exit(tsk, code);
kcov_task_exit(tsk);
kmsan_task_exit(tsk);

@@ -866,7 +867,6 @@ void __noreturn do_exit(long code)

if (group_dead)
acct_process();
- trace_sched_process_exit(tsk);

exit_sem(tsk);
exit_shm(tsk);
--
2.25.1



2024-02-26 20:15:58

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH v2] exit: add exit_code to trace_sched_process_exit and move it earlier in do_exit()

Well. since I have already participated in the previous discussions...

As I said, I can't ack this (user-visible) patch even if I tried to
suggest this from the very beginning, I leave it to the maintainers.

I see nothing wrong in this change, but let me ask: do we really need
to report the exit code? this makes this patch even more user-visible
and I have no idea if it can break the current users.

On 02/23, [email protected] wrote:
>
> From: Wen Yang <[email protected]>
>
> Currently coredump_task_exit() takes some time to wait for the generation
> of the dump file. But if the user-space wants to receive a notification
> as soon as possible it maybe inconvenient.
>
> Add exit_code to the TP trace_sched_process_exit() and move it earlier in
> do_exit(). This way a user-space monitor could detect the exits and
> potentially make some preparations in advance.
>
> Suggested-by: Steven Rostedt <[email protected]>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Mathieu Desnoyers <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: [email protected]
> ---
> include/trace/events/sched.h | 28 +++++++++++++++++++++++++---
> kernel/exit.c | 2 +-
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
> index dbb01b4b7451..c2e8655fd453 100644
> --- a/include/trace/events/sched.h
> +++ b/include/trace/events/sched.h
> @@ -330,9 +330,31 @@ DEFINE_EVENT(sched_process_template, sched_process_free,
> /*
> * Tracepoint for a task exiting:
> */
> -DEFINE_EVENT(sched_process_template, sched_process_exit,
> - TP_PROTO(struct task_struct *p),
> - TP_ARGS(p));
> +TRACE_EVENT(sched_process_exit,
> +
> + TP_PROTO(struct task_struct *task, long code),
> +
> + TP_ARGS(task, code),
> +
> + TP_STRUCT__entry(
> + __array( char, comm, TASK_COMM_LEN )
> + __field( pid_t, pid )
> + __field( int, prio )
> + __field( long, code )
> + ),
> +
> + TP_fast_assign(
> + memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> + __entry->pid = task->pid;
> + __entry->prio = task->prio;
> + __entry->code = code;
> + ),
> +
> + TP_printk("comm=%s pid=%d prio=%d exit_code=0x%lx",
> + __entry->comm, __entry->pid, __entry->prio,
> + __entry->code)
> +);
> +
>
> /*
> * Tracepoint for waiting on task to unschedule:
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 493647fd7c07..48b6ed7f7760 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -826,6 +826,7 @@ void __noreturn do_exit(long code)
>
> WARN_ON(tsk->plug);
>
> + trace_sched_process_exit(tsk, code);
> kcov_task_exit(tsk);
> kmsan_task_exit(tsk);
>
> @@ -866,7 +867,6 @@ void __noreturn do_exit(long code)
>
> if (group_dead)
> acct_process();
> - trace_sched_process_exit(tsk);
>
> exit_sem(tsk);
> exit_shm(tsk);
> --
> 2.25.1
>


2024-03-05 15:18:54

by Wen Yang

[permalink] [raw]
Subject: Re: [PATCH v2] exit: add exit_code to trace_sched_process_exit and move it earlier in do_exit()



On 2024/2/27 03:44, Oleg Nesterov wrote:
> Well. since I have already participated in the previous discussions...
>
> As I said, I can't ack this (user-visible) patch even if I tried to
> suggest this from the very beginning, I leave it to the maintainers.
>
> I see nothing wrong in this change, but let me ask: do we really need
> to report the exit code? this makes this patch even more user-visible
> and I have no idea if it can break the current users.
>


Sorry, I forgot to add a "Suggested by".

The original purpose of reporting the exit code here is to facilitate
users in obtaining signals that cause the process to exit.

We will make modifications according to your comments and do not report
the exit code, as the kernel module can also obtain signals and only
requires a trace point.



--
Best wishes,
Wen


> On 02/23, [email protected] wrote:
>>
>> From: Wen Yang <[email protected]>
>>
>> Currently coredump_task_exit() takes some time to wait for the generation
>> of the dump file. But if the user-space wants to receive a notification
>> as soon as possible it maybe inconvenient.
>>
>> Add exit_code to the TP trace_sched_process_exit() and move it earlier in
>> do_exit(). This way a user-space monitor could detect the exits and
>> potentially make some preparations in advance.
>>
>> : Steven Rostedt <[email protected]>
>> Signed-off-by: Wen Yang <[email protected]>
>> Cc: Steven Rostedt <[email protected]>
>> Cc: Oleg Nesterov <[email protected]>
>> Cc: Masami Hiramatsu <[email protected]>
>> Cc: Mathieu Desnoyers <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Mel Gorman <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: [email protected]
>> ---
>> include/trace/events/sched.h | 28 +++++++++++++++++++++++++---
>> kernel/exit.c | 2 +-
>> 2 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
>> index dbb01b4b7451..c2e8655fd453 100644
>> --- a/include/trace/events/sched.h
>> +++ b/include/trace/events/sched.h
>> @@ -330,9 +330,31 @@ DEFINE_EVENT(sched_process_template, sched_process_free,
>> /*
>> * Tracepoint for a task exiting:
>> */
>> -DEFINE_EVENT(sched_process_template, sched_process_exit,
>> - TP_PROTO(struct task_struct *p),
>> - TP_ARGS(p));
>> +TRACE_EVENT(sched_process_exit,
>> +
>> + TP_PROTO(struct task_struct *task, long code),
>> +
>> + TP_ARGS(task, code),
>> +
>> + TP_STRUCT__entry(
>> + __array( char, comm, TASK_COMM_LEN )
>> + __field( pid_t, pid )
>> + __field( int, prio )
>> + __field( long, code )
>> + ),
>> +
>> + TP_fast_assign(
>> + memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
>> + __entry->pid = task->pid;
>> + __entry->prio = task->prio;
>> + __entry->code = code;
>> + ),
>> +
>> + TP_printk("comm=%s pid=%d prio=%d exit_code=0x%lx",
>> + __entry->comm, __entry->pid, __entry->prio,
>> + __entry->code)
>> +);
>> +
>>
>> /*
>> * Tracepoint for waiting on task to unschedule:
>> diff --git a/kernel/exit.c b/kernel/exit.c
>> index 493647fd7c07..48b6ed7f7760 100644
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -826,6 +826,7 @@ void __noreturn do_exit(long code)
>>
>> WARN_ON(tsk->plug);
>>
>> + trace_sched_process_exit(tsk, code);
>> kcov_task_exit(tsk);
>> kmsan_task_exit(tsk);
>>
>> @@ -866,7 +867,6 @@ void __noreturn do_exit(long code)
>>
>> if (group_dead)
>> acct_process();
>> - trace_sched_process_exit(tsk);
>>
>> exit_sem(tsk);
>> exit_shm(tsk);
>> --
>> 2.25.1
>>
>