LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov@parallels.com>
To: Christoph Lameter <cl@linux.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.cz>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -mm 1/3] slub: don't fail kmem_cache_shrink if slab placement optimization fails
Date: Mon, 26 Jan 2015 22:36:29 +0300	[thread overview]
Message-ID: <20150126193629.GA2660@esperanza> (raw)
In-Reply-To: <alpine.DEB.2.11.1501261216120.16638@gentwo.org>

On Mon, Jan 26, 2015 at 12:24:49PM -0600, Christoph Lameter wrote:
> On Mon, 26 Jan 2015, Vladimir Davydov wrote:
> 
> > Anyways, I think that silently relying on the fact that the allocator
> > never fails small allocations is kind of unreliable. What if this
> 
> We are not doing that though. If the allocation fails we do nothing.

Yeah, that's correct, but memcg/kmem wants it to always free empty slabs
(see patch 3 for details), so I'm trying to be punctual and eliminate
any possibility of failure, because a failure (if it ever happened)
would result in a permanent memory leak (pinned mem_cgroup + its
kmem_caches).

> 
> > > > +			if (page->inuse < objects)
> > > > +				list_move(&page->lru,
> > > > +					  slabs_by_inuse + page->inuse);
> > > >  			if (!page->inuse)
> > > >  				n->nr_partial--;
> > > >  		}
> > >
> > > The condition is always true. A page that has page->inuse == objects
> > > would not be on the partial list.
> > >
> >
> > This is in case we failed to allocate the slabs_by_inuse array. We only
> > have a list for empty slabs then (on stack).
> 
> Ok in that case objects == 1. If you want to do this maybe do it in a more
> general way?
> 
> You could allocate an array on the stack to deal with the common cases. I
> believe an array of 32 objects would be fine to allocate and cover most of
> the slab caches on the system? Would eliminate most of the allocations in
> kmem_cache_shrink.

We could do that, but IMO that would only complicate the code w/o
yielding any real benefits. This function is slow and called rarely
anyway, so I don't think there is any point to optimize out a page
allocation here.

Thanks,
Vladimir

  reply	other threads:[~2015-01-26 19:36 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 12:55 [PATCH -mm 0/3] slub: make dead caches discard free slabs immediately Vladimir Davydov
2015-01-26 12:55 ` [PATCH -mm 1/3] slub: don't fail kmem_cache_shrink if slab placement optimization fails Vladimir Davydov
2015-01-26 15:48   ` Christoph Lameter
2015-01-26 17:01     ` Vladimir Davydov
2015-01-26 18:24       ` Christoph Lameter
2015-01-26 19:36         ` Vladimir Davydov [this message]
2015-01-26 19:53           ` Christoph Lameter
2015-01-27 12:58             ` Vladimir Davydov
2015-01-27 17:02               ` Christoph Lameter
2015-01-28 15:00                 ` Vladimir Davydov
2015-01-26 12:55 ` [PATCH -mm 2/3] slab: zap kmem_cache_shrink return value Vladimir Davydov
2015-01-26 15:49   ` Christoph Lameter
2015-01-26 17:04     ` Vladimir Davydov
2015-01-26 18:26       ` Christoph Lameter
2015-01-26 19:48         ` Vladimir Davydov
2015-01-26 19:55           ` Christoph Lameter
2015-01-26 20:16             ` Vladimir Davydov
2015-01-26 20:28               ` Christoph Lameter
2015-01-26 20:43                 ` Vladimir Davydov
2015-01-26 12:55 ` [PATCH -mm 3/3] slub: make dead caches discard free slabs immediately Vladimir Davydov
2015-01-27  8:00   ` Joonsoo Kim
2015-01-27  8:23     ` Vladimir Davydov
2015-01-27  9:21       ` Joonsoo Kim
2015-01-27  9:28         ` 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=20150126193629.GA2660@esperanza \
    --to=vdavydov@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.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;
as well as URLs for NNTP newsgroup(s).