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 F22D4C2BD09 for ; Thu, 27 Jun 2024 11:54:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 438446B008A; Thu, 27 Jun 2024 07:54:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3E8E66B008C; Thu, 27 Jun 2024 07:54:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2889B6B0092; Thu, 27 Jun 2024 07:54:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 07E796B008A for ; Thu, 27 Jun 2024 07:54:29 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 77E0AC01A2 for ; Thu, 27 Jun 2024 11:54:28 +0000 (UTC) X-FDA: 82276511016.22.1AE34A3 Received: from mail-lf1-f46.google.com (mail-lf1-f46.google.com [209.85.167.46]) by imf24.hostedemail.com (Postfix) with ESMTP id 4D59A18000C for ; Thu, 27 Jun 2024 11:54:26 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=CqgnmJGR; spf=pass (imf24.hostedemail.com: domain of mhocko@suse.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719489249; 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=LmlCKFAQTumozZSb2qpyhtZvbivJ59JtxtsMbo9O2jU=; b=JUbdwdBJc0ZqWnLxxVr739emo17l99vySHfnxIzl0kRmLvSyU0uYaMk2em2TVluVLmrlGd XDgzfe4fFqQ1RefVLHdTqXCB88wmJJk7s12sjzkOUAeGuwbsPU0K785jPSjUpM+qXj80fQ rV6UCD+TZEvJMqi6YfvqVAxUfohpEAc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719489249; a=rsa-sha256; cv=none; b=A3BGzaIj3Z0tsWBbOlsGjPgyWQz/n/rghputmfClu5OuCFgbGyzR14YQ+bICSc9nuDGycx olt7E9wOM1wJN5o74Axaixptk+kPzvaqWLXOzZ9EKXOq00dJdWpLp8WGopg5g8z6IbLw1u WtNGYCA2TcY2EDEPM8pQQnfF+ci10G8= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=CqgnmJGR; spf=pass (imf24.hostedemail.com: domain of mhocko@suse.com designates 209.85.167.46 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com Received: by mail-lf1-f46.google.com with SMTP id 2adb3069b0e04-52ce9ba0cedso5865207e87.2 for ; Thu, 27 Jun 2024 04:54:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1719489264; x=1720094064; 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=LmlCKFAQTumozZSb2qpyhtZvbivJ59JtxtsMbo9O2jU=; b=CqgnmJGRcd0KCSaTInWuFqYEAwmMBTJK8kcIdshNB8ykhSINB1VeEa1p1vAUXnoQ/6 GFMOMRxVTDRSIQihKjCJoFIQ529bTBY13KXuwZCRha8H1O9LAokqvbExTtFC69ys1fvL QZD9rU9NefLxaloat94UhBzeHfJyPL46UbvNBFCF3a3LlTeIFsBC441RpXbouPADmy45 bt4gXthu5twlS5O19S1GyFZ0b/uNXOkhdc0Id1qn7V0T/KxXa4XYgEoNXPIqhZw4Z/E8 9y6viU7IXbkOy4LXbVQpugORELFKZetpwk7Ff7HN0Jobui9dhU9SrOItK2o2q9jLVcPb 1+iQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719489264; x=1720094064; 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=LmlCKFAQTumozZSb2qpyhtZvbivJ59JtxtsMbo9O2jU=; b=S3tcOHLFybqljeNLgGJRbF8A0valDWTltX1FqOjTk3i0wmWFnnhMl0TH30yV4f/puB iVsAVv+cRSrxvaxOfjKbACA9PrpawVBc69kjIKnF5cgEbAM+/VJzHcBKuyeQuB6He9tp RJ1OnbFKAyVdf13A59Fqt6xPC/rflJ7UlTpYQRirnnTdgsrTxh27mkwI0zQdgKhHLGkl HgUtz0Duf05NohX2EZUomM3qPmC9yOM+PcqCAfpIKEPTJsu7zwIRWZ8uc6VX6ALxRDhm zMSZhYrem8a57u2d+hRLI7FuQ3rG0e5peayuznvhF705o/KqtJXzc29k9nTSZqOIUdSt uzdg== X-Forwarded-Encrypted: i=1; AJvYcCVAqIhQpzTmAk4/W/6yoHCMnRmEBTu1NCQJ7fzKPVldZCwqG+0Gam/X4bl2ZuRV/cAYhL/iaIJSZVWxAyFSY94r09I= X-Gm-Message-State: AOJu0Yxg8ltECNuSaH68fa5nKyW5QA63aMhpL5bslLxmbtJxqY1vZMP5 I9vS0RLmfSgbY8VB6w5RhaFpkh5GS4E/QgQrQv6h/CY7ufdLFYy7qShSidKRtmnCdA4sgJk8uqe U X-Google-Smtp-Source: AGHT+IF/7J1Nf5P8IK2DBJMJhVj+ERVjEaTHp3eq1jikjna1dLR4odCLnSjwp6wO7JK6t38d8jeHJQ== X-Received: by 2002:a05:6512:3d1d:b0:52d:3ada:4b6b with SMTP id 2adb3069b0e04-52d3ada4c32mr6180399e87.1.1719489264372; Thu, 27 Jun 2024 04:54:24 -0700 (PDT) Received: from localhost (nat2.prg.suse.com. [195.250.132.146]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a729d7c957esm52172666b.197.2024.06.27.04.54.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jun 2024 04:54:24 -0700 (PDT) Date: Thu, 27 Jun 2024 13:54:23 +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: <10b948cd-5fbf-78e7-c3e8-6867661fa50b@huawei.com> X-Stat-Signature: 9asbp5d8qh8n13ekyffpurtqw976c3we X-Rspamd-Queue-Id: 4D59A18000C X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1719489266-929491 X-HE-Meta: U2FsdGVkX194BiPEw2ykTXiQYP2DrxNfxotGGLMzi0dZzRPTPbHNQJUgTtmQ9w1+zxAzo9b9ZHSTAy30Pg2l+dFEXurQ86+gACL5NTJ4eXrlGP//oJQWWniSaUNPL/csI+n/B7p0HNaPLq1Yx74ZJ+pgdHObcpP5W3Kq3Kx7qWwMPa+5FnH+YzYu3qMVMJGCtw4sK7bAs7dBscbZfX0F3q10/9rkCW+k/+DBUbWIdqYDz758AFxxtpn0zIqzdcH8tVgczzxgNAzjaLgEEvb8FKJrzWaXDJKZO/yd7dhNsPZ0UQqLrUz+pY1LzYQ6wsl6tdho6MBO/FO6r8IH36l+WVY5K5l6bEq8xvBi7+wFLrdai9HlsmxNP0sqlAgMYMMJ2a31p/4EqjwjGnPhmWTOQFXl5hrfoC+PkSx1IUN83Mk54uAAkJLMjMF6zUwltH/76t77rJALyH70ggEaXH+PTkhFY4d9LEi2atj55mbrQC+k1UCVlQdquYMnH2/DQaT9wpS6L2bxT8FxBVOLt+Uut6NiixzaKFvqzCtzc1z19ZbR3Ys0Cdx4BbMQL94Mb78VrRshy/f1Wx3gfMVCAeX1vX7Te1MEz7lAudfHQg3nSpdFEPhvJe8lbKPlpg0tElOZk24i/BRosHeflaMV6gkXnPiTlMPVXzWy0hdzY48zDAZTOA0oEppdzJRm8jDWdWJPB1sA2ry8eSkN+0P3JGrUpH4o54NbEr32oVVqFTfB5QFaHh6NZ+wDTDsK6nvEwmt+6g+pNpyOy92Tay5Ua2chMWfs6igsdCsUR/YlzBpVCXW9Mqa0N2sJ+L+Q6lEzHwdDszJpaxNDBbFBuftl0jfBpjr4Gwbn1oD6zWF/UvyFqPFTGw4uscKPf2Nfee6xzflYMQvCfexHsmPse5W+bmBbwy9kOeFLyKJdHXphusWpFTxNoQTRZ5uq8fCLTdSId3C4FdRYOBnnI4mxk4xtwhW 6i5bsgBV EIRgT9RXmty1XWTEs5O3as2ArvkfFfYp77nZl9Q1o2bcCblu8RYbHLAHZgQ1tBdkPPvpspwcpifSuYXs1GsKSGBRODDmKSBc0x8kcmhAr9lRn0IE5kxQUFhgx+9jO3dzUxMTXQQ3EyiIBb12CzfDFoMfPXZUmfeOg5cFOKuZDMJVUMFXiDP7qQOuvfeeQiHf1Yee++ChTWsQ8XH5sArso6u/99EK/bdOJWzlCKHguBHeOa/wH7wCJOH41avFp8/fPkO+A3VrdFXe6ARO5ZJCCpzSP5NSk4OTs7ads1lcLVaCHbl4+OlgF+b74t9hMa1tbsLuVnVlsTj+dHn0= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 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