linux-kernel - Re: [PATCH v4 3/3] mm/vmscan: don't demote if there is not enough free memory in the lower memory tier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260127220003.3993576-1-joshua.hahnjy@gmail.com>
Date: Tue, 27 Jan 2026 17:00:02 -0500
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Akinobu Mita <akinobu.mita@...il.com>
Cc: Michal Hocko <mhocko@...e.com>,
	linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	akpm@...ux-foundation.org,
	axelrasmussen@...gle.com,
	yuanchu@...gle.com,
	weixugc@...gle.com,
	hannes@...xchg.org,
	david@...nel.org,
	zhengqi.arch@...edance.com,
	shakeel.butt@...ux.dev,
	lorenzo.stoakes@...cle.com,
	Liam.Howlett@...cle.com,
	vbabka@...e.cz,
	rppt@...nel.org,
	surenb@...gle.com,
	ziy@...dia.com,
	matthew.brost@...el.com,
	rakie.kim@...com,
	byungchul@...com,
	gourry@...rry.net,
	ying.huang@...ux.alibaba.com,
	apopple@...dia.com,
	bingjiao@...gle.com,
	jonathan.cameron@...wei.com,
	pratyush.brahma@....qualcomm.com
Subject: Re: [PATCH v4 3/3] mm/vmscan: don't demote if there is not enough free memory in the lower memory tier

> > > Therefore, it appears that the behavior of get_swappiness() is important
> > > in this issue.
> >
> > This is quite mysterious.
> >
> > Especially because get_swappiness() is an MGLRU exclusive function, I find
> > it quite strange that the issue you mention above occurs regardless of whether
> > MGLRU is enabled or disabled. With MGLRU disabled, did you see the same hangs
> > as before? Were these hangs similarly fixed by modifying the callsite in
> > get_swappiness?
> 
> Good point.
> When MGLRU is disabled, changing only the behavior of can_demote()
> called by get_swappiness() did not solve the problem.
> 
> Instead, the problem was avoided by changing only the behavior of
> can_demote() called by can_reclaim_anon_page(), without changing the
> behavior of can_demote() called from other places.
> 
> > On a separate note, I feel a bit uncomfortable for making this the default
> > setting, regardless of whether there is swap space or not. Just as it is
> > easy to create a degenerate scenario where all memory is unreclaimable
> > and the system starts going into (wasteful) reclaim on the lower tiers,
> > it is equally easy to create a scenario where all memory is very easily
> > reclaimable (say, clean pagecache) and we OOM without making any attempt to
> > free up memory on the lower tiers.
> >
> > Reality is likely somewhere in between. And from my perspective, as long as
> > we have some amount of easily reclaimable memory, I don't think immediately
> > OOMing will be helpful for the system (and even if none of the memory is
> > easily reclaimable, we should still try doing something before killing).
> >
> > > > > The reason for this issue is that memory allocations do not directly
> > > > > trigger the oom-killer, assuming that if the target node has an underlying
> > > > > memory tier, it can always be reclaimed by demotion.
> >
> > This patch enforces that the opposite of this assumption is true; that even
> > if a target node has an underlying memory tier, it can never be reclaimed by
> > demotion.
> >
> > Certainly for systems with swap and some compression methods (z{ram, swap}),
> > this new enforcement could be harmful to the system. What do you think?
> 
> Thank you for the detailed explanation.
> 
> I understand the concern regarding the current patch, which only
> checks the free memory of the demotion target node.
> I will explore a solution.

Hello Akinobu, I hope you had a great weekend!

I noticed something that I thought was worth flagging. It seems like the
primary addition of this patch, which is to check for zone_watermark_ok
across the zones, is already a part of should_reclaim_retry():

    /*
     * Keep reclaiming pages while there is a chance this will lead
     * somewhere.  If none of the target zones can satisfy our allocation
     * request even if all reclaimable pages are considered then we are
     * screwed and have to go OOM.
     */
    for_each_zone_zonelist_nodemask(zone, z, ac->zonelist,
                ac->highest_zoneidx, ac->nodemask) {

	[...snip...]

        /*
         * Would the allocation succeed if we reclaimed all
         * reclaimable pages?
         */
        wmark = __zone_watermark_ok(zone, order, min_wmark,
                ac->highest_zoneidx, alloc_flags, available);

        if (wmark) {
            ret = true;
            break;
        }
    }

... which is called in __alloc_pages_slowpath. I wonder why we don't already
hit this. It seems to do the same thing your patch is doing?

What do you think? I hope you have a great day!
Joshua