linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Glauber Costa <glommer@parallels.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kamezawa.hiroyu@jp.fujitsu.com,
	Johannes Weiner <hannes@cmpxchg.org>, Tejun Heo <tj@kernel.org>,
	Michal Hocko <mhocko@suse.cz>, Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	Suleiman Souhlal <suleiman@google.com>
Subject: Re: [PATCH v6 25/29] memcg/sl[au]b: shrink dead caches
Date: Wed, 7 Nov 2012 08:13:08 +0100	[thread overview]
Message-ID: <509A0A04.2030503@parallels.com> (raw)
In-Reply-To: <20121105164813.2eba5ecb.akpm@linux-foundation.org>

On 11/06/2012 01:48 AM, Andrew Morton wrote:
> On Thu,  1 Nov 2012 16:07:41 +0400
> Glauber Costa <glommer@parallels.com> wrote:
> 
>> This means that when we destroy a memcg cache that happened to be empty,
>> those caches may take a lot of time to go away: removing the memcg
>> reference won't destroy them - because there are pending references, and
>> the empty pages will stay there, until a shrinker is called upon for any
>> reason.
>>
>> In this patch, we will call kmem_cache_shrink for all dead caches that
>> cannot be destroyed because of remaining pages. After shrinking, it is
>> possible that it could be freed. If this is not the case, we'll schedule
>> a lazy worker to keep trying.
> 
> This patch is really quite nasty.  We poll the cache once per minute
> trying to shrink then free it?  a) it gives rise to concerns that there
> will be scenarios where the system could suffer unlimited memory windup
> but mainly b) it's just lame.
> 
> The kernel doesn't do this sort of thing.  The kernel tries to be
> precise: in a situation like this we keep track of the number of
> outstanding objects and when that falls to zero, we free their
> container synchronously.  If those objects are normally left floating
> around in an allocated but reclaimable state then we can address that
> by synchronously freeing them if their container has been destroyed.
> 
> Or something like that.  If it's something else then fine, but not this.
> 
> What do we need to do to fix this?
> 
The original patch had a unlikely() test in the free path, conditional
on whether or not the cache is dead, that would then call this is the
cache would now be empty.

I got several requests to remove it and change it to something like
this, because that is a fast path (I myself think an unlikely branch is
not that bad)

If you think such a test is acceptable, I can bring it back and argue in
the basis of "akpm made me do it!". But meanwhile I will give this extra
though to see if there is any alternative way I can do it...

--
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:[~2012-11-07  7:13 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-01 12:07 [PATCH v6 00/29] kmem controller for memcg Glauber Costa
2012-11-01 12:07 ` [PATCH v6 01/29] memcg: Make it possible to use the stock for more than one page Glauber Costa
2012-11-01 12:07 ` [PATCH v6 02/29] memcg: Reclaim when more than one page needed Glauber Costa
2012-11-01 12:07 ` [PATCH v6 03/29] memcg: change defines to an enum Glauber Costa
2012-11-01 12:07 ` [PATCH v6 04/29] kmem accounting basic infrastructure Glauber Costa
2012-11-01 12:07 ` [PATCH v6 05/29] Add a __GFP_KMEMCG flag Glauber Costa
2012-11-01 19:58   ` Christoph Lameter
2012-11-01 12:07 ` [PATCH v6 06/29] memcg: kmem controller infrastructure Glauber Costa
2012-11-01 20:03   ` Christoph Lameter
2012-11-01 12:07 ` [PATCH v6 07/29] mm: Allocate kernel pages to the right memcg Glauber Costa
2012-11-01 12:07 ` [PATCH v6 08/29] res_counter: return amount of charges after res_counter_uncharge Glauber Costa
2012-11-01 12:07 ` [PATCH v6 09/29] memcg: kmem accounting lifecycle management Glauber Costa
2012-11-01 12:07 ` [PATCH v6 10/29] memcg: use static branches when code not in use Glauber Costa
2012-11-01 12:07 ` [PATCH v6 11/29] memcg: allow a memcg with kmem charges to be destructed Glauber Costa
2012-11-02  0:05   ` Andrew Morton
2012-11-02  7:50     ` Glauber Costa
2012-11-06 10:54     ` Michal Hocko
2012-11-01 12:07 ` [PATCH v6 12/29] execute the whole memcg freeing in free_worker Glauber Costa
2012-11-01 12:07 ` [PATCH v6 13/29] protect architectures where THREAD_SIZE >= PAGE_SIZE against fork bombs Glauber Costa
2012-11-01 12:07 ` [PATCH v6 14/29] Add documentation about the kmem controller Glauber Costa
2012-11-01 12:07 ` [PATCH v6 15/29] slab/slub: struct memcg_params Glauber Costa
2012-11-01 12:07 ` [PATCH v6 16/29] slab: annotate on-slab caches nodelist locks Glauber Costa
2012-11-01 12:07 ` [PATCH v6 17/29] consider a memcg parameter in kmem_create_cache Glauber Costa
2012-11-01 12:07 ` [PATCH v6 18/29] Allocate memory for memcg caches whenever a new memcg appears Glauber Costa
2012-11-06  0:23   ` Andrew Morton
2012-11-07  7:05     ` Glauber Costa
2012-11-07  7:10       ` Andrew Morton
2012-11-01 12:07 ` [PATCH v6 19/29] memcg: infrastructure to match an allocation to the right cache Glauber Costa
2012-11-06  0:28   ` Andrew Morton
2012-11-06  8:03     ` Michal Hocko
2012-11-08 11:05       ` Michal Hocko
2012-11-08 14:33         ` Michal Hocko
2012-11-07  7:04     ` Glauber Costa
2012-11-07  7:13       ` Andrew Morton
2012-11-01 12:07 ` [PATCH v6 20/29] memcg: skip memcg kmem allocations in specified code regions Glauber Costa
2012-11-06  0:33   ` Andrew Morton
2012-11-01 12:07 ` [PATCH v6 21/29] sl[au]b: always get the cache from its page in kmem_cache_free Glauber Costa
2012-11-01 12:07 ` [PATCH v6 22/29] sl[au]b: Allocate objects from memcg cache Glauber Costa
2012-11-01 12:07 ` [PATCH v6 23/29] memcg: destroy memcg caches Glauber Costa
2012-11-02  0:05   ` Andrew Morton
2012-11-02  7:46     ` Glauber Costa
2012-11-02 20:19       ` Michal Hocko
2012-11-06  0:40   ` Andrew Morton
2012-11-01 12:07 ` [PATCH v6 24/29] memcg/sl[au]b Track all the memcg children of a kmem_cache Glauber Costa
2012-11-01 12:07 ` [PATCH v6 25/29] memcg/sl[au]b: shrink dead caches Glauber Costa
2012-11-06  0:48   ` Andrew Morton
2012-11-07  7:13     ` Glauber Costa [this message]
2012-11-07  7:16       ` Andrew Morton
2012-11-07  9:22         ` Glauber Costa
2012-11-07 22:46           ` Andrew Morton
2012-11-08  7:13             ` Glauber Costa
2012-11-08 17:15             ` Christoph Lameter
2012-11-08 19:21               ` Andrew Morton
2012-11-08 22:31                 ` Glauber Costa
2012-11-08 22:40                   ` Andrew Morton
2012-11-09 20:06                     ` Christoph Lameter
2012-11-09 20:04                 ` Christoph Lameter
2012-11-01 12:07 ` [PATCH v6 26/29] Aggregate memcg cache values in slabinfo Glauber Costa
2012-11-06  0:57   ` Andrew Morton
2012-11-01 12:07 ` [PATCH v6 27/29] slab: propagate tunables values Glauber Costa
2012-11-01 12:07 ` [PATCH v6 28/29] slub: slub-specific propagation changes Glauber Costa
2012-11-06 19:25   ` Andrew Morton
2012-11-07 15:53   ` Sasha Levin
2012-11-08  6:51     ` Glauber Costa
2012-11-09  3:37       ` Sasha Levin
2012-11-14 12:06         ` Glauber Costa
2012-11-01 12:07 ` [PATCH v6 29/29] Add slab-specific documentation about the kmem controller Glauber Costa
2012-11-02  0:04 ` [PATCH v6 00/29] kmem controller for memcg Andrew Morton
2012-11-02  7:41   ` Glauber Costa
2012-11-02 19:25     ` JoonSoo Kim
2012-11-02 23:06       ` Tejun Heo
2012-11-05  8:14         ` Glauber Costa
2012-11-05  8:18       ` Glauber Costa
2012-11-03  3:36     ` Greg Thelen
2012-11-02  8:30   ` Pekka Enberg

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=509A0A04.2030503@parallels.com \
    --to=glommer@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=penberg@cs.helsinki.fi \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=suleiman@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox