Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 15 Jun 2021 14:42:35 +0200
From:   Michal Hocko <mhocko@suse.com>
To:     Aaron Tomlin <atomlin@redhat.com>
Cc:     linux-mm@kvack.org, akpm@linux-foundation.org, vbabka@suse.cz,
        penguin-kernel@i-love.sakura.ne.jp, llong@redhat.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm/oom_kill: show oom eligibility when displaying the
 current memory state of all tasks
Message-ID: <YMigO5N55QhnrB87@dhcp22.suse.cz>
References: <20210612204634.1102472-1-atomlin@redhat.com>
 <YMb6w/bGdhJvOy6j@dhcp22.suse.cz>
 <20210615115147.gp3w5bcjoaarvyse@ava.usersys.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210615115147.gp3w5bcjoaarvyse@ava.usersys.com>
Precedence: bulk

On Tue 15-06-21 12:51:47, Aaron Tomlin wrote:
> On Mon 2021-06-14 08:44 +0200, Michal Hocko wrote:
> > Well, I have to say that I have a bit hard time understand the problem
> > statement here. First of all you are very likely basing your observation
> > on an old kernel which is missing a fix which should make the situation
> > impossible IIRC. You should be focusing on a justification why the new
> > information is helpful for the current tree.
> 
> Michal,
> 
> Not exactly.
> 
> See oom_reap_task(). Let's assume an OOM event occurred within the context
> of a memcg and 'memory.oom.group' was not set. If I understand correctly,
> once all attempts to OOM reap the specified task were "unsuccessful" then
> MMF_OOM_SKIP is applied; and, the assumption is it will be terminated
> shorty due to the pending fatal signal (see __oom_kill_process()) i.e. a
> SIGKILL is sent to the "victim" before the OOM reaper is notified. Now
> assuming the above task did not exited yet, another task, in the same
> memcg, could also trigger an OOM event.  Therefore, when showing potential
> OOM victims the task above with MMF_OOM_SKIP set, will indeed be displayed.
> 
> I understanding the point on OOM_SCORE_ADJ_MIN. This can be easily
> identified and is clear to the viewer. However, the same cannot be stated
> for MMF_OOM_SKIP.

This is all true but it is not really clear why that is really a
problem. Kernel log already contains information about reaped processes
as they are reported to the log. I fully acknowledge that this is rather
spartan but on the other hand from years of experience reading oom
reports I have to say the dump_tasks is the least interesting part of
the report (while being the most verbose one).

All that being said, I am not really opposing extending the information
although I am a bit worried about leaking too much internal state to the
log. What I am asking for here is a justification why this addition is a
general improvement and how it helps uderstanding oom reports further.
So please focus on that part.
-- 
Michal Hocko
SUSE Labs