Received-SPF: pass (google.com: domain of linux-kernel+bounces-92575-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <tencent_A3F43166353FC8679F2A05EA9E20A549A107@qq.com>
Date: Tue, 5 Mar 2024 22:58:46 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v2] exit: add exit_code to trace_sched_process_exit and
 move it earlier in do_exit()
To: Oleg Nesterov <oleg@redhat.com>, Steven Rostedt <rostedt@goodmis.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
 Masami Hiramatsu <mhiramat@kernel.org>, Ingo Molnar <mingo@kernel.org>,
 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
 Mel Gorman <mgorman@techsingularity.net>,
 Peter Zijlstra <peterz@infradead.org>, linux-kernel@vger.kernel.org
References: <tencent_20932DE952CC4B7E12CF2E5530D45641BF08@qq.com>
 <20240226194434.GC9510@redhat.com>
Content-Language: en-US
From: Wen Yang <wenyang.linux@foxmail.com>
In-Reply-To: <20240226194434.GC9510@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


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
>>
>