All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tejun Heo <tj@kernel.org>, Roman Gushchin <guro@fb.com>,
	linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 7/7] mm: memcontrol: consolidate lruvec stat flushing
Date: Fri, 5 Feb 2021 12:10:28 -0500	[thread overview]
Message-ID: <YB18BF8b41RPra5b@cmpxchg.org> (raw)
In-Reply-To: <YB1hhwVybr0x5M2j@dhcp22.suse.cz>

On Fri, Feb 05, 2021 at 04:17:27PM +0100, Michal Hocko wrote:
> On Tue 02-02-21 13:47:46, Johannes Weiner wrote:
> > There are two functions to flush the per-cpu data of an lruvec into
> > the rest of the cgroup tree: when the cgroup is being freed, and when
> > a CPU disappears during hotplug. The difference is whether all CPUs or
> > just one is being collected, but the rest of the flushing code is the
> > same. Merge them into one function and share the common code.
> 
> IIUC the only reason for the cpu == -1 special case is to avoid
> zeroying, right? Is this optimization worth the special case? The code
> would be slightly easier to follow without this.

Hm, it was less about the optimization and more about which CPU(s)
need(s) to be handled. But it's pretty silly the way it's written,
indeed. I'll move the for_each_online_cpu() to the caller and drop the
cpu==-1 special casing, it makes things much simpler and more obvious.

> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> Anyway the above is not really a fundamental objection. It is more important
> to unify the flushing.
> 
> Acked-by: Michal Hocko <mhocko@suse.com>

Thanks. v2 is different, so I'll wait with taking the ack.

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
To: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>
Cc: Andrew Morton
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-team-b10kYP2dOMg@public.gmane.org
Subject: Re: [PATCH 7/7] mm: memcontrol: consolidate lruvec stat flushing
Date: Fri, 5 Feb 2021 12:10:28 -0500	[thread overview]
Message-ID: <YB18BF8b41RPra5b@cmpxchg.org> (raw)
In-Reply-To: <YB1hhwVybr0x5M2j-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>

On Fri, Feb 05, 2021 at 04:17:27PM +0100, Michal Hocko wrote:
> On Tue 02-02-21 13:47:46, Johannes Weiner wrote:
> > There are two functions to flush the per-cpu data of an lruvec into
> > the rest of the cgroup tree: when the cgroup is being freed, and when
> > a CPU disappears during hotplug. The difference is whether all CPUs or
> > just one is being collected, but the rest of the flushing code is the
> > same. Merge them into one function and share the common code.
> 
> IIUC the only reason for the cpu == -1 special case is to avoid
> zeroying, right? Is this optimization worth the special case? The code
> would be slightly easier to follow without this.

Hm, it was less about the optimization and more about which CPU(s)
need(s) to be handled. But it's pretty silly the way it's written,
indeed. I'll move the for_each_online_cpu() to the caller and drop the
cpu==-1 special casing, it makes things much simpler and more obvious.

> > Signed-off-by: Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
> 
> Anyway the above is not really a fundamental objection. It is more important
> to unify the flushing.
> 
> Acked-by: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org>

Thanks. v2 is different, so I'll wait with taking the ack.

  reply	other threads:[~2021-02-05 19:38 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-02 18:47 [PATCH 0/7]: mm: memcontrol: switch to rstat Johannes Weiner
2021-02-02 18:47 ` Johannes Weiner
2021-02-02 18:47 ` [PATCH 1/7] mm: memcontrol: fix cpuhotplug statistics flushing Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:23   ` Shakeel Butt
2021-02-02 22:23     ` Shakeel Butt
2021-02-02 22:23     ` Shakeel Butt
2021-02-02 23:07   ` Roman Gushchin
2021-02-02 23:07     ` Roman Gushchin
2021-02-03  2:28     ` Roman Gushchin
2021-02-03  2:28       ` Roman Gushchin
2021-02-04 19:29       ` Johannes Weiner
2021-02-04 19:29         ` Johannes Weiner
2021-02-04 19:34         ` Roman Gushchin
2021-02-04 19:34           ` Roman Gushchin
2021-02-05 17:50           ` Johannes Weiner
2021-02-05 17:50             ` Johannes Weiner
2021-02-04 13:28   ` Michal Hocko
2021-02-04 13:28     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 2/7] mm: memcontrol: kill mem_cgroup_nodeinfo() Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:24   ` Shakeel Butt
2021-02-02 22:24     ` Shakeel Butt
2021-02-02 22:24     ` Shakeel Butt
2021-02-02 23:13   ` Roman Gushchin
2021-02-02 23:13     ` Roman Gushchin
2021-02-04 13:29   ` Michal Hocko
2021-02-04 13:29     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 3/7] mm: memcontrol: privatize memcg_page_state query functions Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 22:26   ` Shakeel Butt
2021-02-02 22:26     ` Shakeel Butt
2021-02-02 23:17   ` Roman Gushchin
2021-02-02 23:17     ` Roman Gushchin
2021-02-04 13:30   ` Michal Hocko
2021-02-04 13:30     ` Michal Hocko
2021-02-02 18:47 ` [PATCH 4/7] cgroup: rstat: support cgroup1 Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-03  1:16   ` Roman Gushchin
2021-02-03  1:16     ` Roman Gushchin
2021-02-04 13:39   ` Michal Hocko
2021-02-04 13:39     ` Michal Hocko
2021-02-04 16:01     ` Johannes Weiner
2021-02-04 16:01       ` Johannes Weiner
2021-02-04 16:42       ` Michal Hocko
2021-02-04 16:42         ` Michal Hocko
2021-02-02 18:47 ` [PATCH 5/7] cgroup: rstat: punt root-level optimization to individual controllers Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-02 18:47 ` [PATCH 6/7] mm: memcontrol: switch to rstat Johannes Weiner
2021-02-02 18:47   ` Johannes Weiner
2021-02-03  1:47   ` Roman Gushchin
2021-02-03  1:47     ` Roman Gushchin
2021-02-04 16:26     ` Johannes Weiner
2021-02-04 16:26       ` Johannes Weiner
2021-02-04 18:45       ` Roman Gushchin
2021-02-04 18:45         ` Roman Gushchin
2021-02-04 20:05         ` Johannes Weiner
2021-02-04 20:05           ` Johannes Weiner
2021-02-04 14:19   ` Michal Hocko
2021-02-04 14:19     ` Michal Hocko
2021-02-04 16:15     ` Johannes Weiner
2021-02-04 16:44       ` Michal Hocko
2021-02-04 16:44         ` Michal Hocko
2021-02-04 20:28         ` Johannes Weiner
2021-02-04 20:28           ` Johannes Weiner
2021-02-05 15:05   ` Michal Hocko
2021-02-05 15:05     ` Michal Hocko
2021-02-05 16:34     ` Johannes Weiner
2021-02-05 16:34       ` Johannes Weiner
2021-02-08 14:07       ` Michal Hocko
2021-02-08 14:07         ` Michal Hocko
2021-02-02 18:47 ` [PATCH 7/7] mm: memcontrol: consolidate lruvec stat flushing Johannes Weiner
2021-02-03  2:25   ` Roman Gushchin
2021-02-03  2:25     ` Roman Gushchin
2021-02-04 21:44     ` Johannes Weiner
2021-02-04 21:44       ` Johannes Weiner
2021-02-04 21:47       ` Roman Gushchin
2021-02-04 21:47         ` Roman Gushchin
2021-02-05 15:17   ` Michal Hocko
2021-02-05 15:17     ` Michal Hocko
2021-02-05 17:10     ` Johannes Weiner [this message]
2021-02-05 17:10       ` Johannes Weiner

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=YB18BF8b41RPra5b@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=guro@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=tj@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.