From: Shakeel Butt <shakeel.butt@linux.dev>
To: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Michal Hocko <mhocko@kernel.org>,
Muchun Song <muchun.song@linux.dev>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/7] memcg: dynamically allocate lruvec_stats
Date: Mon, 29 Apr 2024 12:46:32 -0700 [thread overview]
Message-ID: <fs26xbxmaylxwpv35pb3aak2ilb352glnabsi3wq4havqzlg4j@62cltkwiup4z> (raw)
In-Reply-To: <Zi_BsznrtoC1N_lq@P9FQF9L96D>
On Mon, Apr 29, 2024 at 08:50:11AM -0700, Roman Gushchin wrote:
> On Fri, Apr 26, 2024 at 05:37:28PM -0700, Shakeel Butt wrote:
[...]
> > +unsigned long lruvec_page_state_local(struct lruvec *lruvec,
> > + enum node_stat_item idx)
> > +{
> > + struct mem_cgroup_per_node *pn;
> > + long x = 0;
> > +
> > + if (mem_cgroup_disabled())
> > + return node_page_state(lruvec_pgdat(lruvec), idx);
> > +
> > + pn = container_of(lruvec, struct mem_cgroup_per_node, lruvec);
> > + x = READ_ONCE(pn->lruvec_stats->state_local[idx]);
> > +#ifdef CONFIG_SMP
> > + if (x < 0)
> > + x = 0;
> > +#endif
>
> Not directly related to your change, but do we still need it? And if yes,
> do we really care about !CONFIG_SMP case enough to justify these #ifdefs?
>
That's a good question and I think this is still needed. Particularly on
large machines with large number of CPUs, we can have a situation where
the flusher is flushing the CPU 100 and in parallel some workload
allocated a lot of pages on, let's say, CPU 0 and freed on CPU 200.
> > + return x;
> > +}
> > +
> > /* Subset of vm_event_item to report for memcg event stats */
> > static const unsigned int memcg_vm_event_stat[] = {
> > PGPGIN,
> > @@ -5492,18 +5546,25 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
> > if (!pn)
> > return 1;
> >
> > + pn->lruvec_stats = kzalloc_node(sizeof(struct lruvec_stats), GFP_KERNEL,
> > + node);
>
> Why not GFP_KERNEL_ACCOUNT?
>
Previously struct lruvec_stats was part of struct mem_cgroup_per_node
and we use GFP_KERNEL to allocate struct mem_cgroup_per_node. I kept the
behavior same and if we want to switch to GFP_KERNEL_ACCOUNT, I think it
should be a separate patch.
Thanks for the review.
Shakeel
next prev parent reply other threads:[~2024-04-29 19:46 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 [this message]
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
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=fs26xbxmaylxwpv35pb3aak2ilb352glnabsi3wq4havqzlg4j@62cltkwiup4z \
--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 \
/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.