All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <dchinner@redhat.com>
To: Christoph Lameter <cl@linux.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Mike Snitzer <snitzer@redhat.com>,
	Pekka Enberg <penberg@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	"dm-devel@redhat.com" <dm-devel@redhat.com>,
	Alasdair G Kergon <agk@redhat.com>, Joe Thornber <ejt@redhat.com>,
	Mikulas Patocka <mpatocka@redhat.com>,
	Vivek Goyal <vgoyal@redhat.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Heinz Mauelshagen <heinzm@redhat.com>,
	linux-mm <linux-mm@kvack.org>
Subject: Re: slab-nomerge (was Re: [git pull] device mapper changes for 4.3)
Date: Sat, 5 Sep 2015 08:46:35 +1000	[thread overview]
Message-ID: <20150904224635.GA2562@devil.localdomain> (raw)
In-Reply-To: <alpine.DEB.2.11.1509040849460.30848@east.gentwo.org>

On Fri, Sep 04, 2015 at 08:55:25AM -0500, Christoph Lameter wrote:
> On Fri, 4 Sep 2015, Dave Chinner wrote:
> 
> > There are generic cases where it hurts, so no justification should
> > be needed for those cases...
> 
> Inodes and dentries have constructors. These slabs are not mergeable and
> will never be because they have cache specific code to be executed on the
> object.

I know - I said as much early on in this discussion. That's one of
the generic cases I'm refering to.

I also said that the fact that they are not merged is really by
chance, not by good management. They are not being merged because of
the constructor, not because they have a shrinker. hell, I even said
that if it comes down to it, we don't even need SLAB_NO_MERGE
because we can create dummy constructors to prevent merging....

> > Really, we don't need some stupidly high bar to jump over here -
> > whether merging should be allowed can easily be answered with a
> > simple question: "Does the slab have a shrinker or does it back a
> > mempool?" If the answer is yes then using SLAB_SHRINKER or
> > SLAB_MEMPOOL to trigger the no-merge case doesn't need any more
> > justification from subsystem maintainers at all.
> 
> The slab shrinkers do not use mergeable slab caches.

Please, go back and read what i've already said.

*Some* shrinkers act on mergable slabs because they have no
constructor. e.g. the xfs_dquot and xfs_buf shrinkers.  I want to
keep them separate just like the inode cache is kept separate
because they have workload based demand peaks in the millions of
objects and LRU based shrinker reclaim, just like inode caches do.

That's what I want SLAB_SHRINKER for - to explicitly tell the slab
cache creation that I have a shrinker on this slab and so it should
not merge it with others. Every slab that has a shrinker should be
marked with this flag - we should not be relying on constructors to
prevent merging of critical slab caches with shrinkers....

I really don't see the issue here - explicitly encoding and
documenting the behaviour we've implicitly been relying on for years
is something we do all the time. Code clarity and documented
behaviour is a *good thing*.

Cheers,

Dave.
-- 
Dave Chinner
dchinner@redhat.com

--
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-09-04 22:46 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-02 23:13 slab-nomerge (was Re: [git pull] device mapper changes for 4.3) Linus Torvalds
2015-09-03  0:48 ` Andrew Morton
2015-09-03  0:53   ` Mike Snitzer
2015-09-03  0:51 ` Mike Snitzer
2015-09-03  0:51   ` Mike Snitzer
2015-09-03  1:21   ` Linus Torvalds
2015-09-03  2:31     ` Mike Snitzer
2015-09-03  3:10       ` Christoph Lameter
2015-09-03  4:55         ` Andrew Morton
2015-09-03  6:09           ` Pekka Enberg
2015-09-03  8:53             ` Dave Chinner
2015-09-03  3:11       ` Linus Torvalds
2015-09-03  6:02     ` Dave Chinner
2015-09-03  6:13       ` Pekka Enberg
2015-09-03 10:29       ` Jesper Dangaard Brouer
2015-09-03 16:19         ` Christoph Lameter
2015-09-04  9:10           ` Jesper Dangaard Brouer
2015-09-04 14:13             ` Christoph Lameter
2015-09-04  6:35         ` Sergey Senozhatsky
2015-09-04  7:01           ` Linus Torvalds
2015-09-04  7:59             ` Sergey Senozhatsky
2015-09-04  9:56               ` Sergey Senozhatsky
2015-09-04 14:05               ` Christoph Lameter
2015-09-04 14:11               ` Linus Torvalds
2015-09-05  2:09                 ` Sergey Senozhatsky
2015-09-05  2:09                   ` Sergey Senozhatsky
2015-09-05 20:33                   ` Linus Torvalds
2015-09-07  8:44                     ` Sergey Senozhatsky
2015-09-08  0:22                       ` Sergey Senozhatsky
2015-09-03 15:02       ` Linus Torvalds
2015-09-04  3:26         ` Dave Chinner
2015-09-04  3:51           ` Linus Torvalds
2015-09-05  0:36             ` Dave Chinner
2015-09-05  0:36               ` Dave Chinner
2015-09-07  9:30             ` Jesper Dangaard Brouer
2015-09-07 20:22               ` Linus Torvalds
2015-09-07 20:22                 ` Linus Torvalds
2015-09-07 21:17                 ` Jesper Dangaard Brouer
2015-09-04 13:55           ` Christoph Lameter
2015-09-04 22:46             ` Dave Chinner [this message]
2015-09-05  0:25               ` Christoph Lameter
2015-09-05  1:16                 ` Dave Chinner

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=20150904224635.GA2562@devil.localdomain \
    --to=dchinner@redhat.com \
    --cc=agk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=dm-devel@redhat.com \
    --cc=ejt@redhat.com \
    --cc=heinzm@redhat.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-mm@kvack.org \
    --cc=mpatocka@redhat.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=samitolvanen@google.com \
    --cc=snitzer@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vgoyal@redhat.com \
    --cc=viresh.kumar@linaro.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.