All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roman Gushchin <guro@fb.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Balbir Singh <bsingharora@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	kernel-team@fb.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: bump PGSTEAL*/PGSCAN*/ALLOCSTALL counters in memcg reclaim
Date: Tue, 30 May 2017 14:21:14 +0100	[thread overview]
Message-ID: <20170530132114.GA28148@castle> (raw)
In-Reply-To: <20170530122436.GE7969@dhcp22.suse.cz>

On Tue, May 30, 2017 at 02:24:36PM +0200, Michal Hocko wrote:
> On Mon 29-05-17 14:01:41, Roman Gushchin wrote:
> > Historically, PGSTEAL*/PGSCAN*/ALLOCSTALL counters were used to
> > account only for global reclaim events, memory cgroup targeted reclaim
> > was ignored.
> > 
> > It doesn't make sense anymore, because the whole reclaim path
> > is designed around cgroups. Also, per-cgroup counters can exceed the
> > corresponding global counters, what can be confusing.
> 
> The whole reclaim is designed around cgroups but the source of the
> memory pressure is different. I agree that checking global_reclaim()
> for PGSTEAL_KSWAPD doesn't make much sense because we are _always_ in
> the global reclaim context but counting ALLOCSTALL even for targetted
> memcg reclaim is more confusing than helpful. We usually consider this
> counter to see whether the kswapd catches up with the memory demand
> and the global direct reclaim is indicator it doesn't. The similar
> applies to other counters as well.
> 
> So I do not think this is correct. What is the problem you are trying to
> solve here anyway.

This is a follow-up patch after the discussion here:
https://lkml.org/lkml/2017/5/16/706.

I can agree with you, that a per-cgroup ALLOCSTALL is something different
from a global one, and it's better to keep them separated.

But what about PGSTEAL*/PGSCAN* counters, isn't it better to make them
reflect __all__ reclaim activity, no matter what was a root cause?

Thanks!

Roman

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Roman Gushchin <guro@fb.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Balbir Singh <bsingharora@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>, <kernel-team@fb.com>,
	<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm: bump PGSTEAL*/PGSCAN*/ALLOCSTALL counters in memcg reclaim
Date: Tue, 30 May 2017 14:21:14 +0100	[thread overview]
Message-ID: <20170530132114.GA28148@castle> (raw)
In-Reply-To: <20170530122436.GE7969@dhcp22.suse.cz>

On Tue, May 30, 2017 at 02:24:36PM +0200, Michal Hocko wrote:
> On Mon 29-05-17 14:01:41, Roman Gushchin wrote:
> > Historically, PGSTEAL*/PGSCAN*/ALLOCSTALL counters were used to
> > account only for global reclaim events, memory cgroup targeted reclaim
> > was ignored.
> > 
> > It doesn't make sense anymore, because the whole reclaim path
> > is designed around cgroups. Also, per-cgroup counters can exceed the
> > corresponding global counters, what can be confusing.
> 
> The whole reclaim is designed around cgroups but the source of the
> memory pressure is different. I agree that checking global_reclaim()
> for PGSTEAL_KSWAPD doesn't make much sense because we are _always_ in
> the global reclaim context but counting ALLOCSTALL even for targetted
> memcg reclaim is more confusing than helpful. We usually consider this
> counter to see whether the kswapd catches up with the memory demand
> and the global direct reclaim is indicator it doesn't. The similar
> applies to other counters as well.
> 
> So I do not think this is correct. What is the problem you are trying to
> solve here anyway.

This is a follow-up patch after the discussion here:
https://lkml.org/lkml/2017/5/16/706.

I can agree with you, that a per-cgroup ALLOCSTALL is something different
from a global one, and it's better to keep them separated.

But what about PGSTEAL*/PGSCAN* counters, isn't it better to make them
reflect __all__ reclaim activity, no matter what was a root cause?

Thanks!

Roman

  reply	other threads:[~2017-05-30 13:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-29 13:01 [PATCH] mm: bump PGSTEAL*/PGSCAN*/ALLOCSTALL counters in memcg reclaim Roman Gushchin
2017-05-29 13:01 ` Roman Gushchin
2017-05-30 12:24 ` Michal Hocko
2017-05-30 12:24   ` Michal Hocko
2017-05-30 13:21   ` Roman Gushchin [this message]
2017-05-30 13:21     ` Roman Gushchin
2017-05-30 13:44     ` Michal Hocko
2017-05-30 13:44       ` Michal Hocko

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=20170530132114.GA28148@castle \
    --to=guro@fb.com \
    --cc=bsingharora@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@gmail.com \
    /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.