linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Vladimir Davydov <vdavydov@virtuozzo.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@kernel.org>,
	linux-mm@kvack.org, cgroups@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/7] mm: memcontrol: charge swap to cgroup2
Date: Thu, 17 Dec 2015 11:09:25 -0500	[thread overview]
Message-ID: <20151217160925.GA24124@cmpxchg.org> (raw)
In-Reply-To: <a6d639c29f845c2da9adaaab536754c714099e92.1450352791.git.vdavydov@virtuozzo.com>

On Thu, Dec 17, 2015 at 03:29:54PM +0300, Vladimir Davydov wrote:
> In the legacy hierarchy we charge memsw, which is dubious, because:
> 
>  - memsw.limit must be >= memory.limit, so it is impossible to limit
>    swap usage less than memory usage. Taking into account the fact that
>    the primary limiting mechanism in the unified hierarchy is
>    memory.high while memory.limit is either left unset or set to a very
>    large value, moving memsw.limit knob to the unified hierarchy would
>    effectively make it impossible to limit swap usage according to the
>    user preference.
> 
>  - memsw.usage != memory.usage + swap.usage, because a page occupying
>    both swap entry and a swap cache page is charged only once to memsw
>    counter. As a result, it is possible to effectively eat up to
>    memory.limit of memory pages *and* memsw.limit of swap entries, which
>    looks unexpected.
> 
> That said, we should provide a different swap limiting mechanism for
> cgroup2.
> 
> This patch adds mem_cgroup->swap counter, which charges the actual
> number of swap entries used by a cgroup. It is only charged in the
> unified hierarchy, while the legacy hierarchy memsw logic is left
> intact.
> 
> The swap usage can be monitored using new memory.swap.current file and
> limited using memory.swap.max.
> 
> Note, to charge swap resource properly in the unified hierarchy, we have
> to make swap_entry_free uncharge swap only when ->usage reaches zero,
> not just ->count, i.e. when all references to a swap entry, including
> the one taken by swap cache, are gone. This is necessary, because
> otherwise swap-in could result in uncharging swap even if the page is
> still in swap cache and hence still occupies a swap entry. At the same
> time, this shouldn't break memsw counter logic, where a page is never
> charged twice for using both memory and swap, because in case of legacy
> hierarchy we uncharge swap on commit (see mem_cgroup_commit_charge).

This was actually an oversight when rewriting swap accounting. It
should have always been uncharged when the swap slot is released.

> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

--
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>

  reply	other threads:[~2015-12-17 16:09 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-17 12:29 [PATCH v2 0/7] Add swap accounting " Vladimir Davydov
2015-12-17 12:29 ` [PATCH v2 1/7] mm: memcontrol: charge swap " Vladimir Davydov
2015-12-17 16:09   ` Johannes Weiner [this message]
2016-01-13 16:44   ` Michal Hocko
2015-12-17 12:29 ` [PATCH v2 2/7] mm: vmscan: pass memcg to get_scan_count() Vladimir Davydov
2016-01-13 16:47   ` Michal Hocko
2015-12-17 12:29 ` [PATCH v2 3/7] mm: memcontrol: replace mem_cgroup_lruvec_online with mem_cgroup_online Vladimir Davydov
2016-01-13 16:47   ` Michal Hocko
2015-12-17 12:29 ` [PATCH v2 4/7] swap.h: move memcg related stuff to the end of the file Vladimir Davydov
2016-01-13 16:48   ` Michal Hocko
2015-12-17 12:29 ` [PATCH v2 5/7] mm: vmscan: do not scan anon pages if memcg swap limit is hit Vladimir Davydov
2016-01-13 16:54   ` Michal Hocko
2015-12-17 12:29 ` [PATCH v2 6/7] mm: free swap cache aggressively if memcg swap is full Vladimir Davydov
2016-01-13 16:59   ` Michal Hocko
2015-12-17 12:30 ` [PATCH v2 7/7] Documentation: cgroup: add memory.swap.{current,max} description Vladimir Davydov
2015-12-17 16:16   ` Johannes Weiner
2015-12-18  2:51   ` Kamezawa Hiroyuki
2015-12-18 15:39     ` Vladimir Davydov
2016-01-13 17:02   ` 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=20151217160925.GA24124@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov@virtuozzo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox