linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Glauber Costa <glommer@parallels.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-mm@kvack.org, xfs@oss.sgi.com
Subject: Re: [RFC, PATCH 00/19] Numa aware LRU lists and shrinkers
Date: Fri, 21 Dec 2012 13:50:01 +1100	[thread overview]
Message-ID: <20121221025001.GC15182@dastard> (raw)
In-Reply-To: <50D2FA58.9030605@parallels.com>

On Thu, Dec 20, 2012 at 03:45:28PM +0400, Glauber Costa wrote:
> On 11/28/2012 03:14 AM, Dave Chinner wrote:
> > Hi Glauber,
> > 
> > Here's a working version of my patchset for generic LRU lists and
> > NUMA-aware shrinkers.
.....
> > There's still a bunch of cleanup work needed. e.g. the LRU list
> > walk/isolation code needs to use enums for the isolate callback
> > return code, there needs to be a generic list_lru_for_each() style
> > function for walking all the objects in the cache (which will allow
> > the list_lru structures to be used for things like the per-sb inode
> > list). Indeed, even the name "list_lru" is probably something that
> > should be changed - I think the list has become more of a general
> > per-node list than it's initial functionality as a scalable LRU list
> > implementation and I can see uses for it outside of LRUs...
> > 
> > Comments, thoughts and flames all welcome.
> > 
> 
> I like the general idea, and after a small PoC on my side, I can say it
> can at least provide us with a good and sound route to solve the
> targetted memcg shrinking problem.
> 
> I've already provided you some small feedback about the interface in the
> specific patches.

*nod*

> But on a broader sense: The only thing that still bothers me personally
> (meaning: it created particular pain points), is the very loose coupling
> between all the elements involved in the shrinking process:
> 
> 1) the shrinker, always present
> 2) the lru, usually present
> 3) the cache, usually present, specially when there is an LRU.
> 
> I of course understand that they are not always present, and when they
> are, they are not in a 1:1 relation.
> 
> But still, it would be nice to be able to register them to one another,
> so that we can easily answer things like:
> 
> "Given a set of caches, what is the set of shrinkers that will shrink them?"
> 
> "What are the lrus that are driven by this shrinker?"
> 
> This would allow me to do things like this:
> 
> * When a per-memcg cache is created (not all of the caches are
> replicated), find the shrinkers that can shrink them.
> 
> * For each shrinker, also replicate the LRUs that are driven by them.
> 
> Does that make any sense to you ?

It certainly does, though I see that as a separate problem to the
one that this patch set solves. i.e. this is an issue related to the
scope and context of a shrinker/LRU couplet, rather than the
implementation of a shrinker/LRU couplet. This patchset addresses
the latter of the two, and I'm pretty sure that I mentioned that the
former was not a problem I am trying to solve yet....

As it is, right now we embed the struct shrinker into the owner
context, and that's how we find the LRU/cache instances that the
shrinker operates on. In the case of per-memcg shrinker
instantiation that fixed relationship does not work.

Further, the struct shrinker has context specific information in it,
like the defered scan count that it carries from one invocation to
the next, so what we end up with is a tightly coupled owner/shrinker
relationship.  That is, a shrinker is really made up of four things:

	- a shrinker definition (set of methods and configuration
	  data)
	- a non-volatile set of data
	- the owner context
	- the LRU/cache to be shrunk

I suspect that a shrinker instance would look something like this:

shrinker_instance {
	non-volatile set of data
	LRU/cache to be shrunk
	pointer to the owner context
	pointer to the shrinker definition
}

But I'm not really sure how to group them sanely, how to know what
shrinkers would need multiple instantiation and when you'd do that
instantiation, or even how an owner context would then do global
operations (e.g empty caches prior to unmount).

I simply don't know what the requirements for such infrastructure
is, so I can't really say much more than this. Hence I think the
first thing to do here is work out and document what such
instantiation, tracking and grouping needs to be able to do before
anything else...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.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:[~2012-12-21  2:50 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-27 23:14 [RFC, PATCH 00/19] Numa aware LRU lists and shrinkers Dave Chinner
2012-11-27 23:14 ` [PATCH 01/19] dcache: convert dentry_stat.nr_unused to per-cpu counters Dave Chinner
2012-11-27 23:14 ` [PATCH 02/19] dentry: move to per-sb LRU locks Dave Chinner
2012-11-27 23:14 ` [PATCH 03/19] dcache: remove dentries from LRU before putting on dispose list Dave Chinner
2012-11-27 23:14 ` [PATCH 04/19] mm: new shrinker API Dave Chinner
2012-11-27 23:14 ` [PATCH 05/19] shrinker: convert superblock shrinkers to new API Dave Chinner
2012-12-20 11:06   ` Glauber Costa
2012-12-21  1:46     ` Dave Chinner
2012-12-21 10:17       ` Glauber Costa
2012-11-27 23:14 ` [PATCH 06/19] list: add a new LRU list type Dave Chinner
2012-11-28 16:10   ` Christoph Hellwig
2012-11-27 23:14 ` [PATCH 07/19] inode: convert inode lru list to generic lru list code Dave Chinner
2012-11-27 23:14 ` [PATCH 08/19] dcache: convert to use new lru list infrastructure Dave Chinner
2012-11-27 23:14 ` [PATCH 09/19] list_lru: per-node " Dave Chinner
2012-12-20 11:21   ` Glauber Costa
2012-12-21  1:54     ` Dave Chinner
2013-01-16 19:21   ` Glauber Costa
2013-01-16 22:55     ` Dave Chinner
2013-01-17  0:35       ` Glauber Costa
2013-01-17  4:22         ` Dave Chinner
2013-01-17 18:21           ` Glauber Costa
2013-01-18  0:10             ` Dave Chinner
2013-01-18  0:14               ` Glauber Costa
2013-01-18  8:11                 ` Dave Chinner
2013-01-18 19:10                   ` Glauber Costa
2013-01-19  0:10                     ` Dave Chinner
2013-01-19  0:13                       ` Glauber Costa
2013-01-18  0:51               ` Glauber Costa
2013-01-18  8:08                 ` Dave Chinner
2013-01-18 19:01                   ` Glauber Costa
2012-11-27 23:14 ` [PATCH 10/19] shrinker: add node awareness Dave Chinner
2012-11-27 23:14 ` [PATCH 11/19] fs: convert inode and dentry shrinking to be node aware Dave Chinner
2012-11-27 23:14 ` [PATCH 12/19] xfs: convert buftarg LRU to generic code Dave Chinner
2012-11-27 23:14 ` [PATCH 13/19] xfs: Node aware direct inode reclaim Dave Chinner
2012-11-27 23:14 ` [PATCH 14/19] xfs: use generic AG walk for background " Dave Chinner
2012-11-27 23:14 ` [PATCH 15/19] xfs: convert dquot cache lru to list_lru Dave Chinner
2012-11-28 16:17   ` Christoph Hellwig
2012-11-27 23:14 ` [PATCH 16/19] fs: convert fs shrinkers to new scan/count API Dave Chinner
2012-11-27 23:14 ` [PATCH 17/19] drivers: convert shrinkers to new count/scan API Dave Chinner
2012-11-28  1:13   ` Chris Wilson
2012-11-28  3:17     ` Dave Chinner
2012-11-28  8:21       ` Glauber Costa
2012-11-28 21:28         ` Dave Chinner
2012-11-29 10:29           ` Glauber Costa
2012-11-29 22:02             ` Dave Chinner
2013-06-07 13:37   ` Konrad Rzeszutek Wilk
2012-11-27 23:14 ` [PATCH 18/19] shrinker: convert remaining shrinkers to " Dave Chinner
2012-11-27 23:14 ` [PATCH 19/19] shrinker: Kill old ->shrink API Dave Chinner
2012-11-29 19:02 ` [RFC, PATCH 00/19] Numa aware LRU lists and shrinkers Andi Kleen
2012-11-29 22:09   ` Dave Chinner
2012-12-20 11:45 ` Glauber Costa
2012-12-21  2:50   ` Dave Chinner [this message]
2012-12-21 10:41     ` Glauber Costa
2013-01-21 16:08 ` Glauber Costa
2013-01-21 23:21   ` Dave Chinner
2013-01-23 14:36     ` Glauber Costa
2013-01-23 23:46       ` 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=20121221025001.GC15182@dastard \
    --to=david@fromorbit.com \
    --cc=glommer@parallels.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=xfs@oss.sgi.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).