The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: SeongJae Park <sj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	 Muchun Song <muchun.song@linux.dev>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Michal Hocko <mhocko@suse.com>,
	cgroups@vger.kernel.org, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org, kernel-team@meta.com,
	stable@vger.kernel.org
Subject: Re: [PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub
Date: Thu, 16 Jul 2026 06:40:23 -0700	[thread overview]
Message-ID: <alje3jH32jS6D4H-@gmail.com> (raw)
In-Reply-To: <20260627000445.85650-1-sj@kernel.org>

On Fri, Jun 26, 2026 at 05:04:45PM -0700, SeongJae Park wrote:
> On Fri, 26 Jun 2026 14:53:20 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > 
> > I prefer this way over the idea to initialize in the caller. For the
> > actual implementation, the protocol is that the thing is initialized
> > when the function returns true. This version of the fix maintains that
> > for the dummy as well:
> 
> I agree.  I also feel the caller code is _slightly_ easier to read as is, than
> adding the initialization there.  If it is initialized there, I would assume it
> will be used somewhere.  But after finding out it is not used for early return
> cases including memcg1_oom_prepare() reuturning false case, I would be confused
> about the inefficiency.  Using a variable after passing its pointer to a
> function depending on the function's return value makes me assume the variable
> will be set inside the function.
> 
> The code is simple enough to read in any way, and my taste is sometimes just
> weird, though.
> 
> Anyway nice fix, thank you!
> 
> Reviewed-by: SeongJae Park <sj@kernel.org>

Thanks! I'll respin with the Reviewed-by tags to reflect that these
points have been addressed.

  reply	other threads:[~2026-07-16 13:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 12:43 [PATCH] mm: memcg: initialize *locked in memcg1_oom_prepare() stub Breno Leitao
2026-06-26 13:56 ` Joshua Hahn
2026-06-26 14:23   ` Breno Leitao
2026-06-26 18:53 ` Johannes Weiner
2026-06-27  0:04   ` SeongJae Park
2026-07-16 13:40     ` Breno Leitao [this message]
2026-06-27  0:25 ` Shakeel Butt

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=alje3jH32jS6D4H-@gmail.com \
    --to=leitao@debian.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=sj@kernel.org \
    --cc=stable@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox