All of lore.kernel.org
 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 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.