Received-SPF: pass (google.com: domain of linux-kernel+bounces-152334-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
Date: Sat, 20 Apr 2024 17:25:04 +0000
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
From: shakeel.butt@linux.dev
Message-ID: <e42db54b94ebf8ca3ee14bb41850184750d2b23c@linux.dev>
TLS-Required: No
Subject: Re: [PATCH] memcg: simple cleanup of stats update functions
To: "Johannes Weiner" <hannes@cmpxchg.org>
Cc: "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
In-Reply-To: <20240420132326.GE1055428@cmpxchg.org>
References: <20240419233949.2326100-1-shakeel.butt@linux.dev>
 <20240420132326.GE1055428@cmpxchg.org>

April 20, 2024 at 6:23 AM, "Johannes Weiner" <hannes@cmpxchg.org> wrote:


>=20
>=20Hi Shakeel,
>=20
>=20On Fri, Apr 19, 2024 at 04:39:49PM -0700, Shakeel Butt wrote:
>=20
>=20>=20
>=20> @@ -2983,10 +2984,6 @@ void mem_cgroup_commit_charge(struct folio *=
folio, struct mem_cgroup *memcg)
> >=20
>=20>=20=20
>=20>=20
>=20>  #ifdef CONFIG_MEMCG_KMEM
> >=20
>=20>=20=20
>=20>=20
>=20>  -/*
> >=20
>=20>  - * mod_objcg_mlstate() may be called with irq enabled, so
> >=20
>=20>  - * mod_memcg_lruvec_state() should be used.
> >=20
>=20>  - */
> >=20
>=20>  static inline void mod_objcg_mlstate(struct obj_cgroup *objcg,
> >=20
>=20>  struct pglist_data *pgdat,
> >=20
>=20>  enum node_stat_item idx, int nr)
> >=20
>=20>  @@ -2994,10 +2991,12 @@ static inline void mod_objcg_mlstate(struc=
t obj_cgroup *objcg,
> >=20
>=20>  struct mem_cgroup *memcg;
> >=20
>=20>  struct lruvec *lruvec;
> >=20
>=20>=20=20
>=20>=20
>=20>  + lockdep_assert_irqs_disabled();
> >=20
>=20>  +
> >=20
>=20>  rcu_read_lock();
> >=20
>=20>  memcg =3D obj_cgroup_memcg(objcg);
> >=20
>=20>  lruvec =3D mem_cgroup_lruvec(memcg, pgdat);
> >=20
>=20>  - mod_memcg_lruvec_state(lruvec, idx, nr);
> >=20
>=20>  + __mod_memcg_lruvec_state(lruvec, idx, nr);
> >=20
>=20>  rcu_read_unlock();
> >=20
>=20>  }
> >=20
>=20
> Best to rename it to __mod_objcg_mlstate() as well to follow the
>=20
>=20naming pattern for whether caller or callee handles IRQ toggling?

Will do shortly in v2.

>=20
>=20Otherwise, looks great to me!
>=20
>=20Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks a lot.