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