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

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

[wenyang.linux]

From: Wen Yang <wenyang.linux@foxmail.com>

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 <rostedt@goodmis.org>
Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org
---
 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

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

[Oleg Nesterov]

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, wenyang.linux@foxmail.com wrote:
>
> From: Wen Yang <wenyang.linux@foxmail.com>
> 
> 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 <rostedt@goodmis.org>
> Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: linux-kernel@vger.kernel.org
> ---
>  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
> 

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

[Wen Yang]

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, wenyang.linux@foxmail.com wrote:
>>
>> From: Wen Yang <wenyang.linux@foxmail.com>
>>
>> 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 <rostedt@goodmis.org>
>> Signed-off-by: Wen Yang <wenyang.linux@foxmail.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Oleg Nesterov <oleg@redhat.com>
>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   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
>>
>