From: Nick Piggin <npiggin@suse.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: Dave Chinner <david@fromorbit.com>,
linux-kernel@vger.kernel.org, xfs@oss.sgi.com,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 1/2] mm: add context argument to shrinker callback
Date: Fri, 23 Apr 2010 02:57:11 +1000 [thread overview]
Message-ID: <20100422165711.GA5683@laptop> (raw)
In-Reply-To: <20100422164247.GA15882@infradead.org>
On Thu, Apr 22, 2010 at 12:42:47PM -0400, Christoph Hellwig wrote:
> On Fri, Apr 23, 2010 at 02:38:01AM +1000, Nick Piggin wrote:
> > I don't understand, it should be implemented like just all the other
> > shrinkers AFAIKS. Like the dcache one that has to shrink multiple
> > superblocks. There is absolutely no requirement for this API change
> > to implement it in XFS.
>
> The dcache shrinker is an example for a complete mess.
I don't know. It's not really caused by not registering multiple
shrinkers. It seems to be caused more by the locking, which is not
going away when you have multiple shrinkers.
The XFS patch seems to be pinning the mount structure when it is
registered, so it would have no such locking/refcounting problems
using a private list AFAIKS.
> > But the shrinker list *is* a global list. The downside of it in the way
> > it was done in the XFS patch is that 1) it is much larger than a simple
> > list head, and 2) not usable by anything other then the shrinker.
>
> It is an existing global list just made more useful. Whenever a driver
> has muliple instances of pool that need shrinking this comes in useful,
> it's not related to filesystems at all.
I would say less useful, because shrinker structure cannot be used
by anything but the shrinker, wheras a private list can be used by
anything, including the applicable shrinker.
--
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>
next prev parent reply other threads:[~2010-04-22 16:57 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-13 0:24 [PATCH 0/2] Context sensitive memory shrinker support Dave Chinner
2010-04-13 0:24 ` [PATCH 1/2] mm: add context argument to shrinker callback Dave Chinner
2010-04-13 8:17 ` KOSAKI Motohiro
2010-04-18 0:15 ` Christoph Hellwig
2010-04-19 14:00 ` Nick Piggin
2010-04-20 0:41 ` Dave Chinner
2010-04-20 8:38 ` Nick Piggin
2010-04-20 10:32 ` Dave Chinner
2010-04-21 8:40 ` Nick Piggin
2010-04-22 16:32 ` Christoph Hellwig
2010-04-22 16:38 ` Nick Piggin
2010-04-22 16:42 ` Christoph Hellwig
2010-04-22 16:57 ` Nick Piggin [this message]
2010-04-23 1:58 ` Dave Chinner
2010-04-28 3:38 ` Dave Chinner
2010-04-28 9:39 ` Avi Kivity
2010-04-28 13:45 ` Dave Chinner
2010-04-13 0:24 ` [PATCH 2/2] xfs: add a shrinker to background inode reclaim 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=20100422165711.GA5683@laptop \
--to=npiggin@suse.de \
--cc=akpm@linux-foundation.org \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--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).