All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov@parallels.com>
To: Christoph Lameter <cl@linux.com>
Cc: hannes@cmpxchg.org, mhocko@suse.cz, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH RFC 1/3] slub: keep full slabs on list for per memcg caches
Date: Thu, 15 May 2014 10:34:42 +0400	[thread overview]
Message-ID: <20140515063441.GA32113@esperanza> (raw)
In-Reply-To: <alpine.DEB.2.10.1405141114040.16512@gentwo.org>

On Wed, May 14, 2014 at 11:16:36AM -0500, Christoph Lameter wrote:
> On Tue, 13 May 2014, Vladimir Davydov wrote:
> 
> > Currently full slabs are only kept on per-node lists for debugging, but
> > we need this feature to reparent per memcg caches, so let's enable it
> > for them too.
> 
> That will significantly impact the fastpaths for alloc and free.
> 
> Also a pretty significant change the logic of the fastpaths since they
> were not designed to handle the full lists. In debug mode all operations
> were only performed by the slow paths and only the slow paths so far
> supported tracking full slabs.

That's the minimal price we have to pay for slab re-parenting, because
w/o it we won't be able to look up for all slabs of a particular per
memcg cache. The question is, can it be tolerated or I'd better try some
other way?

> 
> > @@ -2587,6 +2610,9 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
> >
> >  			} else { /* Needs to be taken off a list */
> >
> > +				if (kmem_cache_has_cpu_partial(s) && !prior)
> > +					new.frozen = 1;
> > +
> >  	                        n = get_node(s, page_to_nid(page));
> 
> Make this code conditional?

No problem, this patch is just a draft. Thanks to static keys, it won't
be difficult to eliminate any overhead if there is no kmem-active
memcgs.

Thanks.

> 
> >  				/*
> >  				 * Speculatively acquire the list_lock.
> > @@ -2606,6 +2632,12 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
> >  		object, new.counters,
> >  		"__slab_free"));
> >
> > +	if (unlikely(n) && new.frozen && !was_frozen) {
> > +		remove_full(s, n, page);
> > +		spin_unlock_irqrestore(&n->list_lock, flags);
> > +		n = NULL;
> > +	}
> > +
> >  	if (likely(!n)) {
> 
> Here too.

--
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: Christoph Lameter <cl@linux.com>
Cc: <hannes@cmpxchg.org>, <mhocko@suse.cz>,
	<akpm@linux-foundation.org>, <linux-kernel@vger.kernel.org>,
	<linux-mm@kvack.org>
Subject: Re: [PATCH RFC 1/3] slub: keep full slabs on list for per memcg caches
Date: Thu, 15 May 2014 10:34:42 +0400	[thread overview]
Message-ID: <20140515063441.GA32113@esperanza> (raw)
In-Reply-To: <alpine.DEB.2.10.1405141114040.16512@gentwo.org>

On Wed, May 14, 2014 at 11:16:36AM -0500, Christoph Lameter wrote:
> On Tue, 13 May 2014, Vladimir Davydov wrote:
> 
> > Currently full slabs are only kept on per-node lists for debugging, but
> > we need this feature to reparent per memcg caches, so let's enable it
> > for them too.
> 
> That will significantly impact the fastpaths for alloc and free.
> 
> Also a pretty significant change the logic of the fastpaths since they
> were not designed to handle the full lists. In debug mode all operations
> were only performed by the slow paths and only the slow paths so far
> supported tracking full slabs.

That's the minimal price we have to pay for slab re-parenting, because
w/o it we won't be able to look up for all slabs of a particular per
memcg cache. The question is, can it be tolerated or I'd better try some
other way?

> 
> > @@ -2587,6 +2610,9 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
> >
> >  			} else { /* Needs to be taken off a list */
> >
> > +				if (kmem_cache_has_cpu_partial(s) && !prior)
> > +					new.frozen = 1;
> > +
> >  	                        n = get_node(s, page_to_nid(page));
> 
> Make this code conditional?

No problem, this patch is just a draft. Thanks to static keys, it won't
be difficult to eliminate any overhead if there is no kmem-active
memcgs.

Thanks.

> 
> >  				/*
> >  				 * Speculatively acquire the list_lock.
> > @@ -2606,6 +2632,12 @@ static void __slab_free(struct kmem_cache *s, struct page *page,
> >  		object, new.counters,
> >  		"__slab_free"));
> >
> > +	if (unlikely(n) && new.frozen && !was_frozen) {
> > +		remove_full(s, n, page);
> > +		spin_unlock_irqrestore(&n->list_lock, flags);
> > +		n = NULL;
> > +	}
> > +
> >  	if (likely(!n)) {
> 
> Here too.

  reply	other threads:[~2014-05-15  6:34 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 13:48 [PATCH RFC 0/3] kmemcg slab reparenting Vladimir Davydov
2014-05-13 13:48 ` Vladimir Davydov
2014-05-13 13:48 ` [PATCH RFC 1/3] slub: keep full slabs on list for per memcg caches Vladimir Davydov
2014-05-13 13:48   ` Vladimir Davydov
2014-05-14 16:16   ` Christoph Lameter
2014-05-14 16:16     ` Christoph Lameter
2014-05-15  6:34     ` Vladimir Davydov [this message]
2014-05-15  6:34       ` Vladimir Davydov
2014-05-15 15:15       ` Christoph Lameter
2014-05-15 15:15         ` Christoph Lameter
2014-05-16 13:06         ` Vladimir Davydov
2014-05-16 13:06           ` Vladimir Davydov
2014-05-16 15:05           ` Christoph Lameter
2014-05-16 15:05             ` Christoph Lameter
2014-05-13 13:48 ` [PATCH RFC 2/3] percpu-refcount: allow to get dead reference Vladimir Davydov
2014-05-13 13:48   ` Vladimir Davydov
2014-05-13 13:48 ` [PATCH RFC 3/3] slub: reparent memcg caches' slabs on memcg offline Vladimir Davydov
2014-05-13 13:48   ` Vladimir Davydov
2014-05-14 16:20   ` Christoph Lameter
2014-05-14 16:20     ` Christoph Lameter
2014-05-15  7:16     ` Vladimir Davydov
2014-05-15  7:16       ` Vladimir Davydov
2014-05-15 15:16       ` Christoph Lameter
2014-05-15 15:16         ` Christoph Lameter
2014-05-16 13:22         ` Vladimir Davydov
2014-05-16 13:22           ` Vladimir Davydov
2014-05-16 15:03           ` Christoph Lameter
2014-05-16 15:03             ` Christoph Lameter
2014-05-19 15:24             ` Vladimir Davydov
2014-05-19 15:24               ` Vladimir Davydov
2014-05-19 16:03               ` Christoph Lameter
2014-05-19 16:03                 ` Christoph Lameter
2014-05-19 18:27                 ` Vladimir Davydov
2014-05-19 18:27                   ` Vladimir Davydov
2014-05-21 13:58                   ` Vladimir Davydov
2014-05-21 13:58                     ` Vladimir Davydov
2014-05-21 14:45                     ` Christoph Lameter
2014-05-21 14:45                       ` Christoph Lameter
2014-05-21 15:14                       ` Vladimir Davydov
2014-05-21 15:14                         ` Vladimir Davydov
2014-05-22  0:15                         ` Christoph Lameter
2014-05-22  0:15                           ` Christoph Lameter
2014-05-22 14:07                           ` Vladimir Davydov
2014-05-22 14:07                             ` Vladimir Davydov
2014-05-21 14:41                   ` Christoph Lameter
2014-05-21 14:41                     ` Christoph Lameter
2014-05-21 15:04                     ` Vladimir Davydov
2014-05-21 15:04                       ` Vladimir Davydov
2014-05-22  0:13                       ` Christoph Lameter
2014-05-22  0:13                         ` Christoph Lameter
2014-05-22 13:47                         ` Vladimir Davydov
2014-05-22 13:47                           ` Vladimir Davydov
2014-05-22 19:25                           ` Christoph Lameter
2014-05-22 19:25                             ` Christoph Lameter
2014-05-23 15:26                             ` Vladimir Davydov
2014-05-23 15:26                               ` Vladimir Davydov
2014-05-23 17:45                               ` Christoph Lameter
2014-05-23 17:45                                 ` Christoph Lameter
2014-05-23 19:57                                 ` Vladimir Davydov
2014-05-23 19:57                                   ` Vladimir Davydov
2014-05-27 14:38                                   ` Christoph Lameter
2014-05-27 14:38                                     ` Christoph Lameter

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=20140515063441.GA32113@esperanza \
    --to=vdavydov@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    /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.