| Message ID | 20250415024532.26632-3-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Eliminate Dying Memory Cgroup | expand |
On Tue, Apr 15, 2025 at 10:45:06AM +0800, Muchun Song wrote: > If a folio isn't charged to the memory cgroup, holding an rcu read lock > is needless. Users only want to know its charge status, so use > folio_memcg_charged() here. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > --- > mm/memcontrol.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 61488e45cab2..0fc76d50bc23 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -797,20 +797,17 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx, > void __lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx, > int val) > { > - struct mem_cgroup *memcg; > pg_data_t *pgdat = folio_pgdat(folio); > struct lruvec *lruvec; > > - rcu_read_lock(); > - memcg = folio_memcg(folio); > - /* Untracked pages have no memcg, no lruvec. Update only the node */ > - if (!memcg) { > - rcu_read_unlock(); > + if (!folio_memcg_charged(folio)) { > + /* Untracked pages have no memcg, no lruvec. Update only the node */ > __mod_node_page_state(pgdat, idx, val); > return; > } > > - lruvec = mem_cgroup_lruvec(memcg, pgdat); > + rcu_read_lock(); > + lruvec = mem_cgroup_lruvec(folio_memcg(folio), pgdat); > __mod_lruvec_state(lruvec, idx, val); > rcu_read_unlock(); Hm, but untracked pages are the rare exception. It would seem better for that case to take the rcu_read_lock() unnecessarily, than it is to look up folio->memcg_data twice in the fast path?
> On Apr 17, 2025, at 22:48, Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Tue, Apr 15, 2025 at 10:45:06AM +0800, Muchun Song wrote: >> If a folio isn't charged to the memory cgroup, holding an rcu read lock >> is needless. Users only want to know its charge status, so use >> folio_memcg_charged() here. >> >> Signed-off-by: Muchun Song <songmuchun@bytedance.com> >> --- >> mm/memcontrol.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index 61488e45cab2..0fc76d50bc23 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -797,20 +797,17 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx, >> void __lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx, >> int val) >> { >> - struct mem_cgroup *memcg; >> pg_data_t *pgdat = folio_pgdat(folio); >> struct lruvec *lruvec; >> >> - rcu_read_lock(); >> - memcg = folio_memcg(folio); >> - /* Untracked pages have no memcg, no lruvec. Update only the node */ >> - if (!memcg) { >> - rcu_read_unlock(); >> + if (!folio_memcg_charged(folio)) { >> + /* Untracked pages have no memcg, no lruvec. Update only the node */ >> __mod_node_page_state(pgdat, idx, val); >> return; >> } >> >> - lruvec = mem_cgroup_lruvec(memcg, pgdat); >> + rcu_read_lock(); >> + lruvec = mem_cgroup_lruvec(folio_memcg(folio), pgdat); >> __mod_lruvec_state(lruvec, idx, val); >> rcu_read_unlock(); > > Hm, but untracked pages are the rare exception. It would seem better > for that case to take the rcu_read_lock() unnecessarily, than it is to > look up folio->memcg_data twice in the fast path? Yep, you are right. I'll drop this next version. Thanks. Muchun, Thanks.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 61488e45cab2..0fc76d50bc23 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -797,20 +797,17 @@ void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx, void __lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx, int val) { - struct mem_cgroup *memcg; pg_data_t *pgdat = folio_pgdat(folio); struct lruvec *lruvec; - rcu_read_lock(); - memcg = folio_memcg(folio); - /* Untracked pages have no memcg, no lruvec. Update only the node */ - if (!memcg) { - rcu_read_unlock(); + if (!folio_memcg_charged(folio)) { + /* Untracked pages have no memcg, no lruvec. Update only the node */ __mod_node_page_state(pgdat, idx, val); return; } - lruvec = mem_cgroup_lruvec(memcg, pgdat); + rcu_read_lock(); + lruvec = mem_cgroup_lruvec(folio_memcg(folio), pgdat); __mod_lruvec_state(lruvec, idx, val); rcu_read_unlock(); }
If a folio isn't charged to the memory cgroup, holding an rcu read lock is needless. Users only want to know its charge status, so use folio_memcg_charged() here. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- mm/memcontrol.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)