linux-kernel - Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 27 Jun 2024 09:13:29 +0200
From: Michal Hocko <mhocko@...e.com>
To: Xiu Jianfeng <xiujianfeng@...wei.com>
Cc: hannes@...xchg.org, roman.gushchin@...ux.dev, shakeel.butt@...ux.dev,
	muchun.song@...ux.dev, akpm@...ux-foundation.org,
	cgroups@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next] mm: memcg: remove redundant
 seq_buf_has_overflowed()

On Wed 26-06-24 09:42:32, Xiu Jianfeng wrote:
> Both the end of memory_stat_format() and memcg_stat_format() will call
> WARN_ON_ONCE(seq_buf_has_overflowed()). However, memory_stat_format()
> is the only caller of memcg_stat_format(), when memcg is on the default
> hierarchy, seq_buf_has_overflowed() will be executed twice, so remove
> the reduntant one.

Shouldn't we rather remove both? Are they giving us anything useful
actually? Would a simpl pr_warn be sufficient? Afterall all we care
about is to learn that we need to grow the buffer size because our stats
do not fit anymore. It is not really important whether that is an OOM or
cgroupfs interface path.

> Signed-off-by: Xiu Jianfeng <xiujianfeng@...wei.com>
> ---
>  mm/memcontrol.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 974bd160838c..776d22bc66a2 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1846,9 +1846,6 @@ static void memcg_stat_format(struct mem_cgroup *memcg, struct seq_buf *s)
>  			       vm_event_name(memcg_vm_event_stat[i]),
>  			       memcg_events(memcg, memcg_vm_event_stat[i]));
>  	}
> -
> -	/* The above should easily fit into one page */
> -	WARN_ON_ONCE(seq_buf_has_overflowed(s));
>  }
>  
>  static void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s);
> -- 
> 2.34.1

-- 
Michal Hocko
SUSE Labs