Re: [PATCH v2] mm/oom_kill: remove unreachable __GFP_THISNODE check in constrained_alloc()

On Thu 13-08-26 17:38:09, Ye Liu wrote:
> From: Ye Liu <[email protected]>
> 
> The __GFP_THISNODE check in constrained_alloc() is dead code: global
> OOM is never triggered with __GFP_THISNODE (blocked in
> __alloc_pages_may_oom before out_of_memory() is called), and memcg OOM
> returns CONSTRAINT_MEMCG at the top of the function before reaching
> this point. Remove the check, its stale comment, and update the
> following comment that referenced __GFP_THISNODE.
> 
> Signed-off-by: Ye Liu <[email protected]>

Acked-by: Michal Hocko <[email protected]>
Thanks!

> ---
> v2:
>  - Remove the dead code directly.
>  - Link: https://lore.kernel.org/all/[email protected]/
> 
>  mm/oom_kill.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 5f372f6e26fa..fd3c476846a3 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -267,18 +267,11 @@ static enum oom_constraint constrained_alloc(struct oom_control *oc)
>  
>  	if (!oc->zonelist)
>  		return CONSTRAINT_NONE;
> -	/*
> -	 * Reach here only when __GFP_NOFAIL is used. So, we should avoid
> -	 * to kill current.We have to random task kill in this case.
> -	 * Hopefully, CONSTRAINT_THISNODE...but no way to handle it, now.
> -	 */
> -	if (oc->gfp_mask & __GFP_THISNODE)
> -		return CONSTRAINT_NONE;
>  
>  	/*
> -	 * This is not a __GFP_THISNODE allocation, so a truncated nodemask in
> -	 * the page allocator means a mempolicy is in effect.  Cpuset policy
> -	 * is enforced in get_page_from_freelist().
> +	 * A truncated nodemask in the page allocator means a mempolicy
> +	 * is in effect.  Cpuset policy is enforced in
> +	 * get_page_from_freelist().
>  	 */
>  	if (oc->nodemask &&
>  	    !nodes_subset(node_states[N_MEMORY], *oc->nodemask)) {
> -- 
> 2.25.1

-- 
Michal Hocko
SUSE Labs