Received-SPF: pass (google.com: domain of linux-kernel+bounces-70469-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Sun, 18 Feb 2024 18:52:04 +0100
From: Oleg Nesterov <oleg@redhat.com>
To: Wen Yang <wenyang.linux@foxmail.com>
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
Subject: Re: [PATCH] coredump debugging: add a tracepoint to report the
 coredumping
Message-ID: <20240218175204.GB24311@redhat.com>
References: <tencent_5CD40341EC9384E9B7CC127EA5CF2655B408@qq.com>
 <20240217104924.GB10393@redhat.com>
 <tencent_6EFB821C2775D38F99EBFC6C9F7FAB82A809@qq.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <tencent_6EFB821C2775D38F99EBFC6C9F7FAB82A809@qq.com>
User-Agent: Mutt/1.5.24 (2015-08-30)

On 02/18, Wen Yang wrote:
>
> On 2024/2/17 18:49, Oleg Nesterov wrote:
> >On 02/17, 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 the new trace_sched_process_coredump() into coredump_task_exit(),
> >>this way a user-space monitor could easily wait for the exits and
> >>potentially make some preparations in advance.
> >Can't comment, I never know when the new tracepoint will make sense.
> >
> >Stupid question.
> >Oleg.
>
> Thanks for your help.

Well thanks, but no, I can't help. As I said I can't really comment this
patch.

> trace_sched_process_exit()?is located after the PF_EXITING flag is set

Yes,

> so it could not be moved to there.

Why? DECLARE_EVENT_CLASS(sched_process_template) doesn't report task->flags.

Again, again, I am not arguing. But I think that the changelog should
explain why we can't move trace_sched_process_exit() in more details.

> Could we make the following modifications?
..
>
> @@ -2866,6 +2866,7 @@ bool get_signal(struct ksignal *ksig)
> ???????????????? * Anything else is fatal, maybe with a core dump.
> ???????????????? */
> ??????????????? current->flags |= PF_SIGNALED;
> +?????????????? trace_sched_process_kill(current);

Another case when I can't comment the intent.

We already have trace_signal_deliver() in get_signal(). I'm afraid you
need to explain why do you think userspace needs yet another tracepoint.

Oleg.