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

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

[Ye Liu]

From: Ye Liu <liuye@kylinos.cn>

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 <liuye@kylinos.cn>
---
v2:
 - Remove the dead code directly.
 - Link: https://lore.kernel.org/all/20260813030654.320269-1-ye.liu@linux.dev/

 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

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

[Michal Hocko]

On Thu 13-08-26 17:38:09, Ye Liu wrote:
> From: Ye Liu <liuye@kylinos.cn>
> 
> 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 <liuye@kylinos.cn>

Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!

> ---
> v2:
>  - Remove the dead code directly.
>  - Link: https://lore.kernel.org/all/20260813030654.320269-1-ye.liu@linux.dev/
> 
>  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