linux-kernel - Re: [PATCH] memcg: avoid dead loop when setting memory.max

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 for Android: free password hash cracker in your pocket

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

Message-ID: <24u7qp3ln2qaenzsdf6y4wimj4cbsylgs37ppex2jbq2hnonnv@m5hnbr7bz2t3>
Date: Tue, 11 Feb 2025 16:29:32 -0800
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Michal Hocko <mhocko@...e.com>
Cc: Chen Ridong <chenridong@...weicloud.com>, hannes@...xchg.org, 
	roman.gushchin@...ux.dev, muchun.song@...ux.dev, akpm@...ux-foundation.org, 
	cgroups@...r.kernel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	chenridong@...wei.com, wangweiyang2@...wei.com
Subject: Re: [PATCH] memcg: avoid dead loop when setting memory.max

On Tue, Feb 11, 2025 at 09:35:47PM +0100, Michal Hocko wrote:
> On Tue 11-02-25 11:04:21, Shakeel Butt wrote:
> > On Tue, Feb 11, 2025 at 08:18:19AM +0000, Chen Ridong wrote:
> [...]
> > Wouldn't it be more robust if we put an upper bound on the else case of
> > above condition i.e. fix number of retries? As you have discovered there
> > is a hidden dependency on the forward progress of oom_reaper and this
> > check/code-path which I think is not needed. 
> 
> Any OOM path has a dependency on oom_reaper or task exiting.

Personally I would say any allocation (or charge) has a dependency on
oom_reaper making progress (but not arguing on this point).

> Is there
> any reason why this path should be any special? With cond_resched we can
> look for a day where this will be just removed and the code will still
> work. With a number of retries we will have a non-deterministic time
> dependent behavior because number of retries rather than fwd progress
> would define the failure mode.

I am not against adding cond_resched() which might/will be removed in
future. To me it is just that we are leaving our fate to cpu scheduler
here which maybe is ok as we (MM) do it all over the place. Anyways no
objection from me.