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:   Thu, 2 Sep 2021 15:34:33 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Michal Hocko <mhocko@suse.com>
Cc:     David Rientjes <rientjes@google.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Christian Brauner <christian@brauner.io>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] mm/oom: detect and kill task which has allocation
 forbidden by cpuset limit
Message-ID: <20210902073433.GA48711@shbuild999.sh.intel.com>
References: <1630399085-70431-1-git-send-email-feng.tang@intel.com>
 <YS5RTiVgydjszmjn@dhcp22.suse.cz>
 <52d80e9-cf27-9a59-94fd-d27a1e2dac6f@google.com>
 <20210901024402.GB46357@shbuild999.sh.intel.com>
 <20210901134200.GA50993@shbuild999.sh.intel.com>
 <YS+ItADNn08hCLPY@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YS+ItADNn08hCLPY@dhcp22.suse.cz>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

On Wed, Sep 01, 2021 at 04:05:40PM +0200, Michal Hocko wrote:
[SNIP]
> 
> This looks better than the previous attempt. It would be still better to
> solve this at the page allocator layer. The slowpath is already doing
> this for the nodemask. E.g.
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index eeb3a9cb36bb..a3193134540d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4929,6 +4929,17 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	if (!ac->preferred_zoneref->zone)
>  		goto nopage;
>  
> +	/*
> +	 * Check for insane configurations where the cpuset doesn't contain any suitable
> +	 * zone to satisfy the request - e.g. kernel allocations from MOVABLE nodes only
> +	 */
> +	if (cpusets_enabled() && (gfp_mask & __GFP_HARDWALL)) {
> +		struct zoneref *z = first_zones_zonelist(ac->zonelist, ac->highest_zoneidx,
> +				&cpuset_current_mems_allowed);
> +		if (!z->zone)
> +			goto nopage;
> +	}
> +
>  	if (alloc_flags & ALLOC_KSWAPD)
>  		wake_all_kswapds(order, gfp_mask, ac);

Thanks for the suggestion! It dose bail out early skipping the kswapd,
direct reclaim and compaction.

I also looked at prepare_alloc_pages() which does some cpuset check
and zone initialization, but I'd better leave it alone as it's in a
real hot path, while here is in slowpath anyway. 

Will run some page fault benchmark cases with this patch.

Thanks,
Feng

> if this is seen as an additional overhead for an insane configuration
> then we can add insane_cpusets_enabled() which would be a static branch
> enabled when somebody actually tries to configure movable only cpusets
> or potentially other dubious usage.
> -- 
> Michal Hocko
> SUSE Labs