Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Fri, 24 Nov 2023 10:30:44 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Andrew Morton <akpm@linux-foundation.org>,
        gaoxu <gaoxu2@hihonor.com>
Cc:     "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Suren Baghdasaryan <surenb@google.com>,
        yipengxiang <yipengxiang@hihonor.com>
Subject: Re: =?utf-8?B?5Zue5aSNOiBbUEFUQ0hdIG1tLG9v?=
 =?utf-8?B?bV9yZWFwZXI6?= avoid run queue_oom_reaper if task is not oom
Message-ID: <ZWBtRGCXbz49UeXa@tiehlicka>
References: <400d13bddb524ef6af37cb2220808c75@hihonor.com>
 <ZV8SenfRYnkKwqu6@tiehlicka>
 <242025e9a8c84f6b96ba3f180ea01be9@hihonor.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <242025e9a8c84f6b96ba3f180ea01be9@hihonor.com>
Precedence: bulk

On Fri 24-11-23 03:15:46, gaoxu wrote:
[...]
> >> [3701:11_see]Unable to handle kernel NULL pointer dereference at 
> >> virtual address 0000000000000328 [3701:11_see]user pgtable: 4k pages, 
> >> 39-bit VAs, pgdp=00000000821de000 [3701:11_see][0000000000000328] 
> >> pgd=0000000000000000,
> >> p4d=0000000000000000,pud=0000000000000000
> >> [3701:11_see]tracing off
> >> [3701:11_see]Internal error: Oops: 96000005 [#1] PREEMPT SMP 
> >> [3701:11_see]Call trace:
> >> [3701:11_see] queue_oom_reaper+0x30/0x170
> >
> > Could you resolve this offset into the code line please?
> Due to the additional code we added for log purposes, the line numbers may not correspond to the original Linux code.
> 
> static void queue_oom_reaper(struct task_struct *tsk)
> {
> 	/* mm is already queued? */
> 	if (test_and_set_bit(MMF_OOM_REAP_QUEUED, &tsk->signal->oom_mm->flags)) //a null pointer exception occurred
> 		return;

Did you manage to narrow it down to which of the dereference this
corresponds to? Is it tsk->signal == NULL or signal->oom_mm == NULL.
The faulting address doesn't match neither with my configs.

[...]

> >> --- a/mm/oom_kill.c
> >> +++ b/mm/oom_kill.c
> >> @@ -984,7 +984,7 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
> >>  	}
> >>  	rcu_read_unlock();
> >>  
> >> -	if (can_oom_reap)
> >> +	if (can_oom_reap && tsk_is_oom_victim(victim))
> >>  		queue_oom_reaper(victim);
> >
> > I do not understand. We always do send SIGKILL and call mark_oom_victim(victim); on victim task when reaching out here. How can tsk_is_oom_victim can ever be false?
> This is a low-probability issue, as it only occurred once during the monkey testing.
> I haven't been able to find the root cause either.

OK, was there any non-standard code running during this test?
In any case I do not see how this patch could be correct. If, for some
reason we managed to release the signal structure or something else then
we need to understand whether this is a locking or reference counting
issue. I do not really see how this would be possible. But this check
right here doesn't really make sense.

Andrew please drop the patch from your tree.
-- 
Michal Hocko
SUSE Labs