Received-SPF: pass (google.com: domain of linux-kernel+bounces-75551-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240111210539.636607-1-carlosgalo@google.com> <ZdWshXSoLthv6J6b@tiehlicka>
In-Reply-To: <ZdWshXSoLthv6J6b@tiehlicka>
From: Carlos Galo <carlosgalo@google.com>
Date: Wed, 21 Feb 2024 13:30:51 -0800
Message-ID: <CABtOLR+gpU2BYxcGCCqccZGcRDF337z3JJ=7nvC47ANHmS1tNA@mail.gmail.com>
Subject: Re: [PATCH v2] mm: Update mark_victim tracepoints fields
To: Michal Hocko <mhocko@suse.com>
Cc: rostedt@goodmis.org, akpm@linux-foundation.org, surenb@google.com, 
	android-mm@google.com, kernel-team@android.com, 
	Masami Hiramatsu <mhiramat@kernel.org>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, 
	linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 20, 2024 at 11:55=E2=80=AFPM Michal Hocko <mhocko@suse.com> wro=
te:
>
> Hi,
> sorry I have missed this before.
>
> On Thu 11-01-24 21:05:30, Carlos Galo wrote:
> > The current implementation of the mark_victim tracepoint provides only
> > the process ID (pid) of the victim process. This limitation poses
> > challenges for userspace tools that need additional information
> > about the OOM victim. The association between pid and the additional
> > data may be lost after the kill, making it difficult for userspace to
> > correlate the OOM event with the specific process.
>
> You are correct that post OOM all per-process information is lost. On
> the other hand we do dump all this information to the kernel log. Could
> you explain why that is not suitable for your purpose?

Userspace tools often need real-time visibility into OOM situations
for userspace intervention. Our use case involves utilizing BPF
programs, along with BPF ring buffers, to provide OOM notification to
userspace. Parsing kernel logs would be significant overhead as
opposed to the event based BPF approach.

> > In order to mitigate this limitation, add the following fields:
> >
> > - UID
> >    In Android each installed application has a unique UID. Including
> >    the `uid` assists in correlating OOM events with specific apps.
> >
> > - Process Name (comm)
> >    Enables identification of the affected process.
> >
> > - OOM Score
> >    Allows userspace to get additional insights of the relative kill
> >    priority of the OOM victim.
>
> What is the oom score useful for?
>
The OOM score provides us a measure of the victim's importance. On the
android side, it allows us to identify if top or foreground apps are
killed, which have user perceptible impact.

> Is there any reason to provide a different information from the one
> reported to the kernel log?
> __oom_kill_process:
> pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-r=
ss:%lukB, shmem-rss:%lukB, UID:%u pgtables:%lukB oom_score_adj:%hd\n",
>                 message, task_pid_nr(victim), victim->comm, K(mm->total_v=
m),
>                 K(get_mm_counter(mm, MM_ANONPAGES)),
>                 K(get_mm_counter(mm, MM_FILEPAGES)),
>                 K(get_mm_counter(mm, MM_SHMEMPAGES)),
>                 from_kuid(&init_user_ns, task_uid(victim)),
>                 mm_pgtables_bytes(mm) >> 10, victim->signal->oom_score_ad=
j);
>

We added these fields we need (UID, process name, and OOM score), but
we're open to adding the others if you prefer that for consistency
with the kernel log.

Thanks,
Carlos

> --
> Michal Hocko
> SUSE Labs