[PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Xiu Jianfeng]

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.

Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.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

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

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@huawei.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

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[xiujianfeng]

On 2024/6/27 15:13, Michal Hocko wrote:
> 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.

I did a test, when I removed both of them and added a lot of prints in
memcg_stat_format() to make the seq_buf overflow, and then cat
memory.stat in user mode, no OOM occurred, and there were no warning
logs in the kernel.

> 
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.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
> 

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

On Thu 27-06-24 16:33:00, xiujianfeng wrote:
> 
> 
> On 2024/6/27 15:13, Michal Hocko wrote:
> > 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.
> 
> I did a test, when I removed both of them and added a lot of prints in
> memcg_stat_format() to make the seq_buf overflow, and then cat
> memory.stat in user mode, no OOM occurred, and there were no warning
> logs in the kernel.

The default buffer size is PAGE_SIZE.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Yosry Ahmed]

On Thu, Jun 27, 2024 at 12:13 AM Michal Hocko <mhocko@suse.com> wrote:
>
> 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.

Is it possible for userspace readers to break if the stats are
incomplete? If yes, I think WARN_ON_ONCE() may be prompted to make it
easier to catch and fix before deployment.

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[xiujianfeng]

On 2024/6/27 19:20, Michal Hocko wrote:
> On Thu 27-06-24 16:33:00, xiujianfeng wrote:
>>
>>
>> On 2024/6/27 15:13, Michal Hocko wrote:
>>> 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.
>>
>> I did a test, when I removed both of them and added a lot of prints in
>> memcg_stat_format() to make the seq_buf overflow, and then cat
>> memory.stat in user mode, no OOM occurred, and there were no warning
>> logs in the kernel.
> 
> The default buffer size is PAGE_SIZE.

Hi Michal,

I'm sorry, I didn't understand what you meant by this sentence. What I
mean is that we can't remove both, otherwise, neither the kernel nor
user space would be aware of a buffer overflow. From my test, there was
no OOM or other exceptions when the overflow occurred; it just resulted
in the displayed information being truncated. Therefore, we need to keep
one.

> 

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

On Thu 27-06-24 19:43:06, xiujianfeng wrote:
> 
> 
> On 2024/6/27 19:20, Michal Hocko wrote:
> > On Thu 27-06-24 16:33:00, xiujianfeng wrote:
> >>
> >>
> >> On 2024/6/27 15:13, Michal Hocko wrote:
> >>> 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.
> >>
> >> I did a test, when I removed both of them and added a lot of prints in
> >> memcg_stat_format() to make the seq_buf overflow, and then cat
> >> memory.stat in user mode, no OOM occurred, and there were no warning
> >> logs in the kernel.
> > 
> > The default buffer size is PAGE_SIZE.
> 
> Hi Michal,
> 
> I'm sorry, I didn't understand what you meant by this sentence. What I
> mean is that we can't remove both, otherwise, neither the kernel nor
> user space would be aware of a buffer overflow. From my test, there was
> no OOM or other exceptions when the overflow occurred; it just resulted
> in the displayed information being truncated. Therefore, we need to keep
> one.

I've had this in mind

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 71fe2a95b8bd..3e17b9c3a27a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1845,9 +1845,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);
@@ -1858,7 +1855,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: Stat buffer insufficient please report\n", __FUNCTION__);
 }
 
 /**

Because WARN_ON_ONCE doesn't buy us anything actually. It will dump
stack trace and it seems really mouthfull (and it will panic when
panic_on_warn is enabled which is likely not a great thing).
-- 
Michal Hocko
SUSE Labs

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

On Thu 27-06-24 04:33:50, Yosry Ahmed wrote:
> On Thu, Jun 27, 2024 at 12:13 AM Michal Hocko <mhocko@suse.com> wrote:
> >
> > 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.
> 
> Is it possible for userspace readers to break if the stats are
> incomplete?

They will certainly get an imprecise picture. Sufficient to break I
dunno.

> If yes, I think WARN_ON_ONCE() may be prompted to make it
> easier to catch and fix before deployment.

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. I do not particularly care about the latter but to me it seems
like the warning is just an over reaction and a simple pr_warn should
just achieve the similar effect - see my other reply
-- 
Michal Hocko
SUSE Labs

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Yosry Ahmed]

On Thu, Jun 27, 2024 at 4:56 AM Michal Hocko <mhocko@suse.com> wrote:
>
> On Thu 27-06-24 04:33:50, Yosry Ahmed wrote:
> > On Thu, Jun 27, 2024 at 12:13 AM Michal Hocko <mhocko@suse.com> wrote:
> > >
> > > 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.
> >
> > Is it possible for userspace readers to break if the stats are
> > incomplete?
>
> They will certainly get an imprecise picture. Sufficient to break I
> dunno.

If some stats go completely missing and a parser expects them to
always be there, I think they may break.

>
> > If yes, I think WARN_ON_ONCE() may be prompted to make it
> > easier to catch and fix before deployment.
>
> The only advantage of WARN_ON_ONCE is that the splat is so verbose that
> it gets noticed.

Right, that's exactly what I meant.

> And also it panics the system if panic_on_warn is
> enabled. I do not particularly care about the latter but to me it seems
> like the warning is just an over reaction and a simple pr_warn should
> just achieve the similar effect - see my other reply

If pr_warn()'s usually get noticed in a timely manner (by testers or
bots), then I think pr_warn() would be sufficient. If they can go
unnoticed for a while, I think WARN_ON_ONCE() may be warranted to
avoid the possibility of breaking a userspace parser.

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

On Thu 27-06-24 05:00:18, Yosry Ahmed wrote:
> On Thu, Jun 27, 2024 at 4:56 AM Michal Hocko <mhocko@suse.com> wrote:
> >
> > On Thu 27-06-24 04:33:50, Yosry Ahmed wrote:
> > > On Thu, Jun 27, 2024 at 12:13 AM Michal Hocko <mhocko@suse.com> wrote:
> > > >
> > > > 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.
> > >
> > > Is it possible for userspace readers to break if the stats are
> > > incomplete?
> >
> > They will certainly get an imprecise picture. Sufficient to break I
> > dunno.
> 
> If some stats go completely missing and a parser expects them to
> always be there, I think they may break.

If they break, we will eventually learn about that with or without
warning. It is true that WARN* is so vocal that people/tooling might
just report that even without breakage but that to me sounds like
abusing WARNING. There were times when this was not a big deal but now
when WARN* are getting CVEs because panic_on_warn this useful debugging
tool has become a new BUG on.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[xiujianfeng]

On 2024/6/27 19:54, Michal Hocko wrote:
> On Thu 27-06-24 19:43:06, xiujianfeng wrote:
>>
>>
>> On 2024/6/27 19:20, Michal Hocko wrote:
>>> On Thu 27-06-24 16:33:00, xiujianfeng wrote:
>>>>
>>>>
>>>> On 2024/6/27 15:13, Michal Hocko wrote:
>>>>> 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.
>>>>
>>>> I did a test, when I removed both of them and added a lot of prints in
>>>> memcg_stat_format() to make the seq_buf overflow, and then cat
>>>> memory.stat in user mode, no OOM occurred, and there were no warning
>>>> logs in the kernel.
>>>
>>> The default buffer size is PAGE_SIZE.
>>
>> Hi Michal,
>>
>> I'm sorry, I didn't understand what you meant by this sentence. What I
>> mean is that we can't remove both, otherwise, neither the kernel nor
>> user space would be aware of a buffer overflow. From my test, there was
>> no OOM or other exceptions when the overflow occurred; it just resulted
>> in the displayed information being truncated. Therefore, we need to keep
>> one.
> 
> I've had this in mind
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 71fe2a95b8bd..3e17b9c3a27a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1845,9 +1845,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);
> @@ -1858,7 +1855,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: Stat buffer insufficient please report\n", __FUNCTION__);

I found that after the change, the effect is as follows:

# dmesg
[   51.028327] memory_stat_format: Stat buffer insufficient please report

with no keywords such as "Failed", "Warning" to draw attention to this
printout. Should we change it to the following?

if (seq_buf_has_overflowed(s))
      pr_warn("%s: Warning, Stat buffer overflow, please report\n",
__FUNCTION__);


>  }
> >  /**
> 
> Because WARN_ON_ONCE doesn't buy us anything actually. It will dump
> stack trace and it seems really mouthfull (and it will panic when
> panic_on_warn is enabled which is likely not a great thing).

Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed()

[Michal Hocko]

On Fri 28-06-24 10:20:23, xiujianfeng wrote:
> 
> 
> On 2024/6/27 19:54, Michal Hocko wrote:
> > On Thu 27-06-24 19:43:06, xiujianfeng wrote:
> >>
> >>
> >> On 2024/6/27 19:20, Michal Hocko wrote:
> >>> On Thu 27-06-24 16:33:00, xiujianfeng wrote:
> >>>>
> >>>>
> >>>> On 2024/6/27 15:13, Michal Hocko wrote:
> >>>>> 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.
> >>>>
> >>>> I did a test, when I removed both of them and added a lot of prints in
> >>>> memcg_stat_format() to make the seq_buf overflow, and then cat
> >>>> memory.stat in user mode, no OOM occurred, and there were no warning
> >>>> logs in the kernel.
> >>>
> >>> The default buffer size is PAGE_SIZE.
> >>
> >> Hi Michal,
> >>
> >> I'm sorry, I didn't understand what you meant by this sentence. What I
> >> mean is that we can't remove both, otherwise, neither the kernel nor
> >> user space would be aware of a buffer overflow. From my test, there was
> >> no OOM or other exceptions when the overflow occurred; it just resulted
> >> in the displayed information being truncated. Therefore, we need to keep
> >> one.
> > 
> > I've had this in mind
> > 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 71fe2a95b8bd..3e17b9c3a27a 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1845,9 +1845,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);
> > @@ -1858,7 +1855,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: Stat buffer insufficient please report\n", __FUNCTION__);
> 
> I found that after the change, the effect is as follows:
> 
> # dmesg
> [   51.028327] memory_stat_format: Stat buffer insufficient please report
> 
> with no keywords such as "Failed", "Warning" to draw attention to this
> printout. Should we change it to the following?
> 
> if (seq_buf_has_overflowed(s))
>       pr_warn("%s: Warning, Stat buffer overflow, please report\n",
> __FUNCTION__);

LGTM.
-- 
Michal Hocko
SUSE Labs