All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov@parallels.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: mhocko@suse.cz, akpm@linux-foundation.org, glommer@gmail.com,
	cl@linux-foundation.org, penberg@kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	devel@openvz.org
Subject: Re: [PATCH RFC -mm v2 0/3] kmemcg: simplify work-flow (was "memcg-vs-slab cleanup")
Date: Fri, 18 Apr 2014 20:04:38 +0400	[thread overview]
Message-ID: <53514D16.80309@parallels.com> (raw)
In-Reply-To: <20140418132331.GA26283@cmpxchg.org>

On 04/18/2014 05:23 PM, Johannes Weiner wrote:
>> First, it removes async per memcg cache destruction (see patches 1, 2).
>> Now caches are only destroyed on memcg offline. That means the caches
>> that are not empty on memcg offline will be leaked. However, they are
>> already leaked, because memcg_cache_params::nr_pages normally never
>> drops to 0 so the destruction work is never scheduled except
>> kmem_cache_shrink is called explicitly. In the future I'm planning
>> reaping such dead caches on vmpressure or periodically.
>
> I like the synchronous handling on css destruction, but the periodical
> reaping part still bothers me.  If there is absolutely 0 use for these
> caches remaining, they shouldn't hang around until we encounter memory
> pressure or a random time interval.

Agree.

> Would it be feasible to implement cache merging in both slub and slab,
> so that upon css destruction the child's cache's remaining slabs could
> be moved to the parent's cache?  If the parent doesn't have one, just
> reparent the whole cache.

Interesting idea. That would definitely look neater than periodic
reaping. But it's going to be an uneasy thing to do I guess, because
synchronization in sl[au]b is a subtle thing. I'll have a closer look at
slab's internals to understand if it's feasible.

>
>> Second, it substitutes per memcg slab_caches_mutex's with the global
>> memcg_slab_mutex, which should be taken during the whole per memcg cache
>> creation/destruction path before the slab_mutex (see patch 3). This
>> greatly simplifies synchronization among various per memcg cache
>> creation/destruction paths.
>
> This sounds reasonable.  I'll go look at the code.

Thank you!

--
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: Vladimir Davydov <vdavydov@parallels.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: <mhocko@suse.cz>, <akpm@linux-foundation.org>,
	<glommer@gmail.com>, <cl@linux-foundation.org>,
	<penberg@kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-mm@kvack.org>, <devel@openvz.org>
Subject: Re: [PATCH RFC -mm v2 0/3] kmemcg: simplify work-flow (was "memcg-vs-slab cleanup")
Date: Fri, 18 Apr 2014 20:04:38 +0400	[thread overview]
Message-ID: <53514D16.80309@parallels.com> (raw)
In-Reply-To: <20140418132331.GA26283@cmpxchg.org>

On 04/18/2014 05:23 PM, Johannes Weiner wrote:
>> First, it removes async per memcg cache destruction (see patches 1, 2).
>> Now caches are only destroyed on memcg offline. That means the caches
>> that are not empty on memcg offline will be leaked. However, they are
>> already leaked, because memcg_cache_params::nr_pages normally never
>> drops to 0 so the destruction work is never scheduled except
>> kmem_cache_shrink is called explicitly. In the future I'm planning
>> reaping such dead caches on vmpressure or periodically.
>
> I like the synchronous handling on css destruction, but the periodical
> reaping part still bothers me.  If there is absolutely 0 use for these
> caches remaining, they shouldn't hang around until we encounter memory
> pressure or a random time interval.

Agree.

> Would it be feasible to implement cache merging in both slub and slab,
> so that upon css destruction the child's cache's remaining slabs could
> be moved to the parent's cache?  If the parent doesn't have one, just
> reparent the whole cache.

Interesting idea. That would definitely look neater than periodic
reaping. But it's going to be an uneasy thing to do I guess, because
synchronization in sl[au]b is a subtle thing. I'll have a closer look at
slab's internals to understand if it's feasible.

>
>> Second, it substitutes per memcg slab_caches_mutex's with the global
>> memcg_slab_mutex, which should be taken during the whole per memcg cache
>> creation/destruction path before the slab_mutex (see patch 3). This
>> greatly simplifies synchronization among various per memcg cache
>> creation/destruction paths.
>
> This sounds reasonable.  I'll go look at the code.

Thank you!

  reply	other threads:[~2014-04-18 16:04 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-18  8:04 [PATCH RFC -mm v2 0/3] kmemcg: simplify work-flow (was "memcg-vs-slab cleanup") Vladimir Davydov
2014-04-18  8:04 ` Vladimir Davydov
2014-04-18  8:04 ` [PATCH RFC -mm v2 1/3] memcg, slab: do not schedule cache destruction when last page goes away Vladimir Davydov
2014-04-18  8:04   ` Vladimir Davydov
2014-04-18 13:41   ` Johannes Weiner
2014-04-18 13:41     ` Johannes Weiner
2014-04-18 16:05     ` Vladimir Davydov
2014-04-18 16:05       ` Vladimir Davydov
2014-04-18  8:04 ` [PATCH RFC -mm v2 2/3] memcg, slab: merge memcg_{bind,release}_pages to memcg_{un}charge_slab Vladimir Davydov
2014-04-18  8:04   ` Vladimir Davydov
2014-04-18 13:44   ` Johannes Weiner
2014-04-18 13:44     ` Johannes Weiner
2014-04-18 16:07     ` Vladimir Davydov
2014-04-18 16:07       ` Vladimir Davydov
2014-04-18  8:04 ` [PATCH RFC -mm v2 3/3] memcg, slab: simplify synchronization scheme Vladimir Davydov
2014-04-18  8:04   ` Vladimir Davydov
2014-04-18 14:17   ` Johannes Weiner
2014-04-18 14:17     ` Johannes Weiner
2014-04-18 16:08     ` Vladimir Davydov
2014-04-18 16:08       ` Vladimir Davydov
2014-04-18 18:26       ` Johannes Weiner
2014-04-18 18:26         ` Johannes Weiner
2014-04-18  8:08 ` [PATCH RFC -mm v2 0/3] kmemcg: simplify work-flow (was "memcg-vs-slab cleanup") Vladimir Davydov
2014-04-18  8:08   ` Vladimir Davydov
2014-04-18 13:23 ` Johannes Weiner
2014-04-18 13:23   ` Johannes Weiner
2014-04-18 16:04   ` Vladimir Davydov [this message]
2014-04-18 16:04     ` Vladimir Davydov
2014-04-20 10:32 ` Vladimir Davydov
2014-04-20 10:32   ` Vladimir Davydov

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=53514D16.80309@parallels.com \
    --to=vdavydov@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux-foundation.org \
    --cc=devel@openvz.org \
    --cc=glommer@gmail.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=penberg@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.