| Message ID | 20240628072333.2496527-1-xiujianfeng@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] mm: memcg: adjust the warning when seq_buf overflows | expand |
On Fri 28-06-24 07:23:33, Xiu Jianfeng wrote: > Currently it uses WARN_ON_ONCE() if seq_buf overflows when user reads > memory.stat, the only advantage of WARN_ON_ONCE is that the splat is > so verbose that it gets noticed. And also it panics the system if > panic_on_warn is enabled. It seems like the warning is just an over > reaction and a simple pr_warn should just achieve the similar effect. > > Suggested-by: Michal Hocko <mhocko@suse.com> > Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Acked-by: Michal Hocko <mhocko@suse.com> I would just squash this with other patch removing it from memcg_stat_format. But this is up to you. Thanks! > --- > mm/memcontrol.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c251bbe35f4b..8e5590ac43d7 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1484,7 +1484,8 @@ static void memory_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) > memcg_stat_format(memcg, s); > else > memcg1_stat_format(memcg, s); > - WARN_ON_ONCE(seq_buf_has_overflowed(s)); > + if (seq_buf_has_overflowed(s)) > + pr_warn("%s: Warning, stat buffer overflow, please report\n", __func__); > } > > /** > -- > 2.34.1 >
On 2024/6/28 15:45, Michal Hocko wrote: > On Fri 28-06-24 07:23:33, Xiu Jianfeng wrote: >> Currently it uses WARN_ON_ONCE() if seq_buf overflows when user reads >> memory.stat, the only advantage of WARN_ON_ONCE is that the splat is >> so verbose that it gets noticed. And also it panics the system if >> panic_on_warn is enabled. It seems like the warning is just an over >> reaction and a simple pr_warn should just achieve the similar effect. >> >> Suggested-by: Michal Hocko <mhocko@suse.com> >> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> > > Acked-by: Michal Hocko <mhocko@suse.com> > > I would just squash this with other patch removing it from > memcg_stat_format. But this is up to you. Thanks, the other patch has already been merged into the -next branch by Andrew. In this situation, I'm not quite clear whether I should send a small separate patch, or squash them and send a v2. If Andrew wants, I can do it. > > Thanks! > >> --- >> mm/memcontrol.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index c251bbe35f4b..8e5590ac43d7 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -1484,7 +1484,8 @@ static void memory_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) >> memcg_stat_format(memcg, s); >> else >> memcg1_stat_format(memcg, s); >> - WARN_ON_ONCE(seq_buf_has_overflowed(s)); >> + if (seq_buf_has_overflowed(s)) >> + pr_warn("%s: Warning, stat buffer overflow, please report\n", __func__); >> } >> >> /** >> -- >> 2.34.1 >> >
On 2024/6/28 15:45, Michal Hocko wrote: > On Fri 28-06-24 07:23:33, Xiu Jianfeng wrote: >> Currently it uses WARN_ON_ONCE() if seq_buf overflows when user reads >> memory.stat, the only advantage of WARN_ON_ONCE is that the splat is >> so verbose that it gets noticed. And also it panics the system if >> panic_on_warn is enabled. It seems like the warning is just an over >> reaction and a simple pr_warn should just achieve the similar effect. >> >> Suggested-by: Michal Hocko <mhocko@suse.com> >> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> > > Acked-by: Michal Hocko <mhocko@suse.com> > > I would just squash this with other patch removing it from > memcg_stat_format. But this is up to you. Sorry, I might have misunderstood, if you can squash them, it looks good to me, thanks. > > Thanks! > >> --- >> mm/memcontrol.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index c251bbe35f4b..8e5590ac43d7 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -1484,7 +1484,8 @@ static void memory_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) >> memcg_stat_format(memcg, s); >> else >> memcg1_stat_format(memcg, s); >> - WARN_ON_ONCE(seq_buf_has_overflowed(s)); >> + if (seq_buf_has_overflowed(s)) >> + pr_warn("%s: Warning, stat buffer overflow, please report\n", __func__); >> } >> >> /** >> -- >> 2.34.1 >> >
On Fri 28-06-24 16:09:02, xiujianfeng wrote: > > > On 2024/6/28 15:45, Michal Hocko wrote: > > On Fri 28-06-24 07:23:33, Xiu Jianfeng wrote: > >> Currently it uses WARN_ON_ONCE() if seq_buf overflows when user reads > >> memory.stat, the only advantage of WARN_ON_ONCE is that the splat is > >> so verbose that it gets noticed. And also it panics the system if > >> panic_on_warn is enabled. It seems like the warning is just an over > >> reaction and a simple pr_warn should just achieve the similar effect. > >> > >> Suggested-by: Michal Hocko <mhocko@suse.com> > >> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> > > > > Acked-by: Michal Hocko <mhocko@suse.com> > > > > I would just squash this with other patch removing it from > > memcg_stat_format. But this is up to you. > > Sorry, I might have misunderstood, if you can squash them, it looks good > to me, thanks. Andrew usually can do that even when the patch is in his tree. But as I've said having 2 patches is ok as well.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index c251bbe35f4b..8e5590ac43d7 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1484,7 +1484,8 @@ static void memory_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) memcg_stat_format(memcg, s); else memcg1_stat_format(memcg, s); - WARN_ON_ONCE(seq_buf_has_overflowed(s)); + if (seq_buf_has_overflowed(s)) + pr_warn("%s: Warning, stat buffer overflow, please report\n", __func__); } /**
Currently it uses WARN_ON_ONCE() if seq_buf overflows when user reads memory.stat, the only advantage of WARN_ON_ONCE is that the splat is so verbose that it gets noticed. And also it panics the system if panic_on_warn is enabled. It seems like the warning is just an over reaction and a simple pr_warn should just achieve the similar effect. Suggested-by: Michal Hocko <mhocko@suse.com> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> --- mm/memcontrol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)