Re: [PATCH] mm/page_alloc: detect allocation forbidden by cpuset and bail out early

From: Michal Hocko <mhocko@suse.com>
To: Feng Tang <feng.tang@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Rientjes <rientjes@google.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/page_alloc: detect allocation forbidden by cpuset and bail out early
Date: Tue, 7 Sep 2021 10:44:32 +0200	[thread overview]
Message-ID: <YTcmcEUmtO++WeBk@dhcp22.suse.cz> (raw)
In-Reply-To: <1631003150-96935-1-git-send-email-feng.tang@intel.com>

On Tue 07-09-21 16:25:50, Feng Tang wrote:
> There was report that starting an Ubuntu in docker while using cpuset
> to bind it to movlabe nodes (a node only has movable zone, like a node

s@movlabe@movable@

> for hotplug or a Persistent Memory  node in normal usage) will fail
> due to memory allocation failure, and then OOM is involved and many
> other innocent processes got killed. It can be reproduced with command:
> $docker run -it --rm  --cpuset-mems 4 ubuntu:latest bash -c
> "grep Mems_allowed /proc/self/status" (node 4 is a movable node)
> 
> The reason is, in the case, the target cpuset nodes only have movable
> zone, while the creation of an OS in docker sometimes needs to allocate
> memory in non-movable zones (dma/dma32/normal) like GFP_HIGHUSER, and
> the cpuset limit forbids the allocation, then out-of-memory killing is
> involved even when normal nodes and movable nodes both have many free
> memory.

It would be great to add a oom report here as an example.

> The failure is reasonable, but still there is one problem, that when
> the usage fails as it's an mission impossible due to the cpuset limit,
> the allocation should just not trigger reclaim/compaction, and more
> importantly, not get any innocent process oom-killed.

I would reformulate to something like:
"
The OOM killer cannot help to resolve the situation as there is no
usable memory for the request in the cpuset scope. The only reasonable
measure to take is to fail the allocation right away and have the caller
to deal with it.
"

> So add detection for cases like this in the slowpath of allocation,
> and bail out early returning NULL for the allocation.
> 
> We've run some cases of malloc/mmap/page_fault/lru-shm/swap from
> will-it-scale and vm-scalability, and didn't see obvious performance
> change (all inside +/- 1%), test boxes are 2 socket Cascade Lake and
> Icelake servers.
> 
> [thanks to Micho Hocko and David Rientjes for suggesting not handle
>  it inside OOM code]

While this is a good fix from the functionality POV I believe you can go
a step further. Please add a detection to the cpuset code and complain
to the kernel log if somebody tries to configure movable only cpuset.
Once you have that in place you can easily create a static branch for
cpuset_insane_setup() and have zero overhead for all reasonable
configuration. There shouldn't be any reason to pay a single cpu cycle
to check for something that almost nobody does.

What do you think?
-- 
Michal Hocko
SUSE Labs