From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF7C1C2BBCA for ; Fri, 28 Jun 2024 07:09:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0929E6B0095; Fri, 28 Jun 2024 03:09:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 043326B0098; Fri, 28 Jun 2024 03:09:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E4BBC6B009A; Fri, 28 Jun 2024 03:09:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id C7EF96B0095 for ; Fri, 28 Jun 2024 03:09:21 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id B992D141597 for ; Fri, 28 Jun 2024 07:09:20 +0000 (UTC) X-FDA: 82279421280.05.1FFBD1A Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf27.hostedemail.com (Postfix) with ESMTP id B090A40017 for ; Fri, 28 Jun 2024 07:09:18 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=JKrdAAnu; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf27.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719558538; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=If0iq5oLy+eiIvzlGQNRLUuZgZ1JLzU6wSKruP4/9Eg=; b=nu7o6BLTXk+hycd6Hy20Vv/N5hzogX22kvhQmvpcsxIV3g7wQ9nK8N6s5hm2XZeEVSRIL0 zhkNCGpNlLqPMNqqJl92oLkinUW0aX7LeL+KsXoesFX6SVJjMbGBdaJUGNhfoCtCvCzK2W gZYh4357iKK6sg4d848NAGiRRLXy4LU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719558538; a=rsa-sha256; cv=none; b=MPfeXO6+DcCOJ21k3keq+VjNjn/BiM+gIav46cYWa8ajozpP/quch4LxlRB5nZ1bsIYu0i RHLKikseir0Ngw311sIfSbUQeL9MjhHXRjdx8su0VhUHvkQ9pF7sHRUN+PNN1GTNMF6rrI HmeBNNu/9bzgsxU6iyBuk0nLyHcY0C0= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=JKrdAAnu; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf27.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=mhocko@suse.com Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-57d2fc03740so274267a12.0 for ; Fri, 28 Jun 2024 00:09:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1719558557; x=1720163357; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=If0iq5oLy+eiIvzlGQNRLUuZgZ1JLzU6wSKruP4/9Eg=; b=JKrdAAnuuZHHP+z/UqwjlZtmSvZgbyTzq4o/D5pxye3Pd6iRTnux2VyRvwuyx6tb+v ZagS02EcnNgipupRA4XjoUyZXqQdA/9jlO/F4kjCAOzAyyz1qUb7NqAHm9Ldij/c86LQ eKDn6uFUqN4IyFbTNVIbL34TDO3mZWij1+G6dc7bVSC5nbt9Dv16yTOo3S+otfkCuzrw g8ATxxcshpkVAnYTCECum91GxOTWX0Z4kWEdHse6uQ/AfzJApXpWB8aT2J4MG3ltCoQA MzWrG9kUJEfkk4Bu5MX1/7U/bJCCZzzbNjI60pGoeafqUhi7fMJhXOHbTFH4rfIe07kL D+/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719558557; x=1720163357; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=If0iq5oLy+eiIvzlGQNRLUuZgZ1JLzU6wSKruP4/9Eg=; b=vXiTBVd2KRS/7IA2NXofpDBO9qVKRWEhdtWnjIXitVLoCii3PAOPhjRuGdAyFi38VN V6j3k04KRBy+VArUTfYzHvBlQ6sN8z2Il5q+GzqNRGxS3DwSnkWqmcWAVxtg5MYJv71c 2nbGOWOovbj2uF33xPnObbU6ccBatV8JLO2p8bi5ywnARmhFPjrhAxeNf9k57X+xrF3P XBl+WxitXp9EyGNqbDQqqRF28UD/oEEDLu59ao17wF2g9h/CX7JW/qIj2B79SMm57XH+ AIXoQxJ29PwUu2Gm5s/IMC4465CO+u88gJu3pMVKSNBmJ738VKgYg6OG92ZOnNFVG5vL sNHQ== X-Forwarded-Encrypted: i=1; AJvYcCViyBuyeKVRekXQaNJaG/H1q3Vb7CAqxgwmF5tc+YjfiUdXY0xYehFkdw1KhZ2SG2zG8JbkWMSUMAkQxOCzrQnuLbg= X-Gm-Message-State: AOJu0YxbKwGoNruq+hNurR9Jx3nK4NSREixoCLWhrAPceuVESoJUiCto 03aFfXQtUggj5x5E5TYREEC+u6tYDBSHDg/G1G+XAPz1qNHOk+wavRXZZd9q+D0= X-Google-Smtp-Source: AGHT+IHI8uJ1o7wEPXjrRlqBMAxE/hZW7hfP6KthuUiDb9dgFhsbRFiAQwu4+lO0LtbeeWla+b5xVw== X-Received: by 2002:a17:906:3383:b0:a72:438b:2dfe with SMTP id a640c23a62f3a-a7245cf26e7mr845900966b.40.1719558556886; Fri, 28 Jun 2024 00:09:16 -0700 (PDT) Received: from localhost (109-81-86-16.rct.o2.cz. [109.81.86.16]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a72ab0651dfsm47940966b.142.2024.06.28.00.09.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Jun 2024 00:09:16 -0700 (PDT) Date: Fri, 28 Jun 2024 09:09:15 +0200 From: Michal Hocko To: xiujianfeng Cc: hannes@cmpxchg.org, roman.gushchin@linux.dev, shakeel.butt@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH -next] mm: memcg: remove redundant seq_buf_has_overflowed() Message-ID: References: <20240626094232.2432891-1-xiujianfeng@huawei.com> <10b948cd-5fbf-78e7-c3e8-6867661fa50b@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: B090A40017 X-Stat-Signature: 5wgun8bsqw1jpf53h3r3uz9n3ra47c8d X-Rspam-User: X-HE-Tag: 1719558558-430366 X-HE-Meta: U2FsdGVkX1/VsfMIqRVIMuZnQp0opFp6EA6LYeNt3gC746ky/IHeUNtiyyZNBFhKFkXqGDg4Nhl6Bs8pSa2fW/vD2GirRT4TJssLugYEkj/JOAGSZJVuVHdu1ELvNz6naA4vf9R1ElEIBgRMQYV/mJihvx2a8bzcSagA4TQsEQnZYg41jisdDGImfSmrwXr+gaOEiZRurGVBBHEGGAxOvwPRLncsROvgzz+kNaemSyfHLYHShxrxYHNGa5VKcYgF3ycJPY6G6ndgz6EwfC+0y1Vd64SGmG2Kf70K+ohIXPY53Nyf0a0n8rAT1Fb1YHxTKwvxY88NT6RaHSSvUfT/waFffRS3pVsHVhtKkmChdEdZKE/u3e09FqcNM8lo3UOv83zks3JpM3OL8AcjtkBpr3t9nCGfpNJ4bN/1SRiNh9Yjv4oDmCI7Fweci4gu95jGeEsQ6+S9+3chbCvXt9/QnG8PJCDskos0vLw3vjCTyaRJBSoT0t3RSU7z8vB+2z+U6aebDj4kKM9IG3T4bIm0YehDP2bu3EmQ/qc/ZHUKg/9GkUJIiqiDVDkfizv8rs4NtAMREkgCfyVDaaKCEc3V5PG9vRo6Mwkm9q3jQqP3tC95ikRRdg83buvM5+Gl7ksFSXyBrGGrFTflX9dd1Vc0QtuRVi/GRg6sDQ4Q7LkoIku2ncPH0nXkFFdj9tbG+HKS5L6ej+rwCK3xb29FM94240CFWd/YOyHlxrAWPfwLOuQoobIDKoRwX1LFCw2emHDvi12WgHp5+EV+eHQa3GxiULQmWoyFrlHNGv7dSGapPZW2SI8O+u4NWPDmAt/ar3DC7WkEgHlTbsGe0U4dfFYh/PqLUgNJSjJW91X1IgnQTw+Dk+auyLDiEfNRnlS4We9w+kL1BQkjyWE0FREk+IDPxe/jug73q5NjfEr72howUWsNGrc7h7ecbl1o0iRHogO3ZTPCrGJxmTb+7h/ZCJz L4fJYHC3 1IUF6wmdraUkm8rW0qAJFrnreKMsav+6xp7yNIkgXdKA+C7fB9zNjvCbXZu+TJ+X1Hb9djMgpv5rKWjGHGq1Alt26TYQKV5mXbzmrwGoK0RcWlkmhRvoSuOZxFMa4dtMOD+2KkSvtp2Mk1N7hdxsgIvgB3SXOo5uS2BUM4yzLSJWPFPCxqY2dZGK7c9y0slp7BSOnQdgweVH+OC74uDdGQfu2hf47bMRI5XTXGOIP+seV3dnWE1m7a4m2apC+YZzn/AytjbWtfmI+zx4bcMSmv4/PImOf4CAffByX3A+mpi8XZuSTuNZP2nt6qvUiSN6fzHkMoBrKNl+WgO4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000009, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: 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