Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 18 Oct 2023 13:37:15 +0200
From:   Michal Hocko <mhocko@suse.com>
To:     Kairui Song <kasong@tencent.com>
Cc:     linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
        Christian Brauner <brauner@kernel.org>,
        Suren Baghdasaryan <surenb@google.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/oom_killer: simplify OOM killer info dump helper
Message-ID: <3hjarpoab5paboraql2ibgqb7hexj2n2fog7rctwgintrvlu24@kwljdjthirws>
References: <20231016113103.86477-1-ryncsn@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231016113103.86477-1-ryncsn@gmail.com>
Precedence: bulk

On Mon 16-10-23 19:31:03, Kairui Song wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> There is only one caller wants to dump the kill victim info, so just let
> it call the standalone helper, no need to make the generic info dump
> helper take an extra argument for that.
> 
> Result of bloat-o-meter:
> ./scripts/bloat-o-meter ./mm/oom_kill.old.o ./mm/oom_kill.o
> add/remove: 0/0 grow/shrink: 1/2 up/down: 131/-142 (-11)
> Function                                     old     new   delta
> oom_kill_process                             412     543    +131
> out_of_memory                               1422    1418      -4
> dump_header                                  562     424    -138
> Total: Before=21514, After=21503, chg -0.05%
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>

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

> ---
>  mm/oom_kill.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 44bde56ecd02..9e6071fde34a 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -437,7 +437,7 @@ static void dump_tasks(struct oom_control *oc)
>  	}
>  }
>  
> -static void dump_oom_summary(struct oom_control *oc, struct task_struct *victim)
> +static void dump_oom_victim(struct oom_control *oc, struct task_struct *victim)
>  {
>  	/* one line summary of the oom killer context. */
>  	pr_info("oom-kill:constraint=%s,nodemask=%*pbl",
> @@ -449,7 +449,7 @@ static void dump_oom_summary(struct oom_control *oc, struct task_struct *victim)
>  		from_kuid(&init_user_ns, task_uid(victim)));
>  }
>  
> -static void dump_header(struct oom_control *oc, struct task_struct *p)
> +static void dump_header(struct oom_control *oc)
>  {
>  	pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, oom_score_adj=%hd\n",
>  		current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order,
> @@ -467,8 +467,6 @@ static void dump_header(struct oom_control *oc, struct task_struct *p)
>  	}
>  	if (sysctl_oom_dump_tasks)
>  		dump_tasks(oc);
> -	if (p)
> -		dump_oom_summary(oc, p);
>  }
>  
>  /*
> @@ -1029,8 +1027,10 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
>  	}
>  	task_unlock(victim);
>  
> -	if (__ratelimit(&oom_rs))
> -		dump_header(oc, victim);
> +	if (__ratelimit(&oom_rs)) {
> +		dump_header(oc);
> +		dump_oom_victim(oc, victim);
> +	}
>  
>  	/*
>  	 * Do we need to kill the entire memory cgroup?
> @@ -1072,7 +1072,7 @@ static void check_panic_on_oom(struct oom_control *oc)
>  	/* Do not panic for oom kills triggered by sysrq */
>  	if (is_sysrq_oom(oc))
>  		return;
> -	dump_header(oc, NULL);
> +	dump_header(oc);
>  	panic("Out of memory: %s panic_on_oom is enabled\n",
>  		sysctl_panic_on_oom == 2 ? "compulsory" : "system-wide");
>  }
> @@ -1155,7 +1155,7 @@ bool out_of_memory(struct oom_control *oc)
>  	select_bad_process(oc);
>  	/* Found nothing?!?! */
>  	if (!oc->chosen) {
> -		dump_header(oc, NULL);
> +		dump_header(oc);
>  		pr_warn("Out of memory and no killable processes...\n");
>  		/*
>  		 * If we got here due to an actual allocation at the
> -- 
> 2.42.0

-- 
Michal Hocko
SUSE Labs