From: Shakeel Butt <shakeel.butt@linux.dev>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Yosry Ahmed <yosryahmed@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
Roman Gushchin <roman.gushchin@linux.dev>,
Muchun Song <muchun.song@linux.dev>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/7] memcg: pr_warn_once for unexpected events and stats
Date: Mon, 29 Apr 2024 12:54:08 -0700 [thread overview]
Message-ID: <5eh7nwj6tnl76bkwnksgbb63mjmvf3rzlaevepcxecl3xhmkc7@hbcmkhseaoky> (raw)
In-Reply-To: <20240427142234.GA1155473@cmpxchg.org>
On Sat, Apr 27, 2024 at 10:22:34AM -0400, Johannes Weiner wrote:
> On Fri, Apr 26, 2024 at 06:18:13PM -0700, Shakeel Butt wrote:
> > On Fri, Apr 26, 2024 at 05:58:16PM -0700, Yosry Ahmed wrote:
> > > On Fri, Apr 26, 2024 at 5:38 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
[...]
> > >
> > > Can we make these more compact by using WARN_ON_ONCE() instead:
> > >
> > > if (WARN_ON_ONCE(i < 0))
> > > return 0;
> > >
> > > I guess the advantage of using pr_warn_once() is that we get to print
> > > the exact stat index, but the stack trace from WARN_ON_ONCE() should
> > > make it obvious in most cases AFAICT.
>
> if (WARN_ONCE(i < 0, "stat item %d not in memcg_node_stat_items\n", i))
> return 0;
>
> should work?
>
> > > No strong opinions either way.
> >
> > One reason I used pr_warn_once() over WARN_ON_ONCE() is the syzbot
> > trigger. No need to trip the bot over this error condition.
>
> The warn splat is definitely quite verbose. But I think that would
> only be annoying initially, in case a site was missed. Down the line,
> it seems helpful to have this stand out to somebody who is trying to
> add a new cgroup stat and forgets to update the right enums.
Sounds good to me. I will change it to WARN_ONCE().
next prev parent reply other threads:[~2024-04-29 19:54 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-27 0:37 [PATCH v2 0/7] memcg: reduce memory consumption by memcg stats Shakeel Butt
2024-04-27 0:37 ` [PATCH v2 1/7] memcg: reduce memory size of mem_cgroup_events_index Shakeel Butt
2024-04-27 0:42 ` Yosry Ahmed
2024-04-27 1:15 ` Shakeel Butt
2024-04-29 15:36 ` Roman Gushchin
2024-04-27 0:37 ` [PATCH v2 2/7] memcg: dynamically allocate lruvec_stats Shakeel Butt
2024-04-27 1:23 ` Yosry Ahmed
2024-04-29 15:50 ` Roman Gushchin
2024-04-29 19:46 ` Shakeel Butt
2024-04-29 21:02 ` Roman Gushchin
2024-04-29 21:59 ` Shakeel Butt
2024-04-27 0:37 ` [PATCH v2 3/7] memcg: reduce memory for the lruvec and memcg stats Shakeel Butt
2024-04-27 0:51 ` Yosry Ahmed
2024-04-27 1:16 ` Shakeel Butt
2024-04-27 1:18 ` Yosry Ahmed
2024-04-29 16:00 ` Roman Gushchin
2024-04-29 20:00 ` Shakeel Butt
2024-04-29 17:35 ` T.J. Mercier
2024-04-29 20:13 ` Shakeel Butt
2024-04-29 22:23 ` T.J. Mercier
2024-04-27 0:37 ` [PATCH v2 4/7] memcg: cleanup __mod_memcg_lruvec_state Shakeel Butt
2024-04-27 0:53 ` Yosry Ahmed
2024-04-29 15:45 ` Roman Gushchin
2024-04-27 0:37 ` [PATCH v2 5/7] memcg: pr_warn_once for unexpected events and stats Shakeel Butt
2024-04-27 0:58 ` Yosry Ahmed
2024-04-27 1:18 ` Shakeel Butt
2024-04-27 14:22 ` Johannes Weiner
2024-04-29 19:54 ` Shakeel Butt [this message]
2024-04-29 16:06 ` Roman Gushchin
2024-04-29 19:56 ` Shakeel Butt
2024-04-27 0:37 ` [PATCH v2 6/7] memcg: use proper type for mod_memcg_state Shakeel Butt
2024-04-27 0:37 ` [PATCH v2 7/7] mm: cleanup WORKINGSET_NODES in workingset Shakeel Butt
2024-04-29 16:07 ` Roman Gushchin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5eh7nwj6tnl76bkwnksgbb63mjmvf3rzlaevepcxecl3xhmkc7@hbcmkhseaoky \
--to=shakeel.butt@linux.dev \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=muchun.song@linux.dev \
--cc=roman.gushchin@linux.dev \
--cc=yosryahmed@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.