From: Nick Piggin <npiggin@kernel.dk>
To: Nick Piggin <npiggin@kernel.dk>
Cc: Dave Chinner <david@fromorbit.com>,
Christoph Hellwig <hch@infradead.org>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 17/18] fs: icache remove inode_lock
Date: Fri, 15 Oct 2010 17:41:50 +1100 [thread overview]
Message-ID: <20101015064150.GB7511@amd> (raw)
In-Reply-To: <20101015034451.GA6827@amd>
On Fri, Oct 15, 2010 at 02:44:51PM +1100, Nick Piggin wrote:
> On Fri, Oct 15, 2010 at 02:30:17PM +1100, Nick Piggin wrote:
> > On Fri, Oct 15, 2010 at 02:13:43PM +1100, Dave Chinner wrote:
> > > You've shown it can be done, and that's great - it shows
> > > us the impact of making those changes, but they need to be analysed
> > > separately and treated on own their merits, not lumped with core
> > > locking changes necessary for store-free path walking.
> >
> > Actually I didn't see anyone else object to doing this. Everybody
> > else it seems acknowledges that it needs to be done, and it gets
> > done naturally as a side effect of fine grained locking.
>
> Let's just get back to this part, which seems to be one you have
> the most issues with maybe?
[snip per-zone lru/locking]
And as far as the rest of the work goes, I much prefer to come
to a basic consensus about the overall design of the entire vfs
scale work, and then focus on the exact implementation and patch
series details. When there is a consensus, I think it makes much
more sense to merge it in quite large chunks.
Ie. all of the inode locking, then all of the dcache locking.
I do not want to just cherry pick things here and there and
leave the others because your particular workload doesn't care
about them, you haven't reviewed them yet, etc. Because that
just gets my plan into a mess.
I'm perfectly fine to change the design, drop some aspects of
it, etc. _if_ it is decided that we don't want them with reasonable
arguments and agreement among everybody. On the other hand I prefer
not to just merge a few bits and leave others out because we
_don't_ have a consensus about one aspect or another.
So if you don't agree with something, let's work out why not and
try to come to an agreement, rather than pushing bits and pieces
that you do happen to agree with.
You're worried about mere mortals reviewing and understanding it...
I don't really know. If you understand inode locking today, you
can understand the inode scaling series quite easily. Ditto for
dcache. rcu-walk path walking is trickier, but it is described in
detail in documentation and changelog.
And you can understand the high level approach without exactly
digesting every detail at once. The inode locking work goes to
break up all global locks:
- a single inode object is protected (to the same level as
inode_lock) with i_lock. This makes it really trivial for
filesystems to lock down the object without taking a global
lock.
- inode hash rcuified and insertion/removal made per-bucket
- inode lru lists and locking made per-zone
- inode sb list made per-sb, per-cpu
- inode counters made per-cpu
- inode io lists and locking made per-bdi
So from the highest level snapshot, this is not rocket science.
And the way I've structured the patches, you can take almost
any of the above points and go look in the patch series to see
how it is implemented.
Is this where we want to go? My argument is yes, and I have
been gradually gathering real results and agreement from others.
I've demonstrated performance improvements, although many of them can
only be actually achieved when dcache, vfsmount, files lock etc scaling
is also implemented, which is another reason why it is so important to
keep everything together. And it's actually not always trivial to just
take a single change and document a performance improvement in
isolation.
But you can use your brain with scalability work, and if you're not
convinced about a particular patch, you can now actually take the
full series and revert a single patch (or add an artifical lock in
there to demonstrate the scalability overhead).
What I have done in the series is required to get almost linear
scalability up to the largest POWER7 system IBM has internally on
almost all important basic vfs operations. It should scale to the
largest UV systems from SGI. And it should scale on -rt. Put a
global lock in inode lru creation/destruction/touch/reclaim path,
and scalability is going to go to hell on these workloads on large
systems again.
And large isn't even that large these days. You can see these
problems clear as day on 2 and 4 socket small servers and we know
it is going to get worse for at least a few more doublings of
core count.
next prev parent reply other threads:[~2010-10-15 6:41 UTC|newest]
Thread overview: 162+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-08 5:21 fs: Inode cache scalability V2 Dave Chinner
2010-10-08 5:21 ` [PATCH 01/18] kernel: add bl_list Dave Chinner
2010-10-08 8:18 ` Andi Kleen
2010-10-08 10:33 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 02/18] fs: Convert nr_inodes and nr_unused to per-cpu counters Dave Chinner
2010-10-08 7:01 ` Christoph Hellwig
2010-10-08 5:21 ` [PATCH 03/18] fs: keep inode with backing-dev Dave Chinner
2010-10-08 7:01 ` Christoph Hellwig
2010-10-08 7:27 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 04/18] fs: Implement lazy LRU updates for inodes Dave Chinner
2010-10-08 7:08 ` Christoph Hellwig
2010-10-08 7:31 ` Dave Chinner
2010-10-08 9:08 ` Al Viro
2010-10-08 9:51 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 05/18] fs: inode split IO and LRU lists Dave Chinner
2010-10-08 7:14 ` Christoph Hellwig
2010-10-08 7:38 ` Dave Chinner
2010-10-08 9:16 ` Al Viro
2010-10-08 9:58 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 06/18] fs: Clean up inode reference counting Dave Chinner
2010-10-08 7:20 ` Christoph Hellwig
2010-10-08 7:46 ` Dave Chinner
2010-10-08 8:15 ` Christoph Hellwig
2010-10-08 5:21 ` [PATCH 07/18] exofs: use iput() for inode reference count decrements Dave Chinner
2010-10-08 7:21 ` Christoph Hellwig
2010-10-16 7:56 ` Nick Piggin
2010-10-16 16:29 ` Christoph Hellwig
2010-10-17 15:41 ` Boaz Harrosh
2010-10-08 5:21 ` [PATCH 08/18] fs: add inode reference coutn read accessor Dave Chinner
2010-10-08 7:24 ` Christoph Hellwig
2010-10-08 5:21 ` [PATCH 09/18] fs: rework icount to be a locked variable Dave Chinner
2010-10-08 7:27 ` Christoph Hellwig
2010-10-08 7:50 ` Dave Chinner
2010-10-08 8:17 ` Christoph Hellwig
2010-10-08 13:16 ` Chris Mason
2010-10-08 9:32 ` Al Viro
2010-10-08 10:15 ` Dave Chinner
2010-10-08 13:14 ` Chris Mason
2010-10-08 13:53 ` Christoph Hellwig
2010-10-08 14:09 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 10/18] fs: Factor inode hash operations into functions Dave Chinner
2010-10-08 7:29 ` Christoph Hellwig
2010-10-08 9:41 ` Al Viro
2010-10-08 5:21 ` [PATCH 11/18] fs: Introduce per-bucket inode hash locks Dave Chinner
2010-10-08 7:33 ` Christoph Hellwig
2010-10-08 7:51 ` Dave Chinner
2010-10-08 9:49 ` Al Viro
2010-10-08 9:51 ` Christoph Hellwig
2010-10-08 13:43 ` Christoph Hellwig
2010-10-08 14:17 ` Dave Chinner
2010-10-08 18:54 ` Christoph Hellwig
2010-10-16 7:57 ` Nick Piggin
2010-10-16 16:16 ` Christoph Hellwig
2010-10-16 17:12 ` Nick Piggin
2010-10-17 0:45 ` Christoph Hellwig
2010-10-17 2:06 ` Nick Piggin
2010-10-17 0:46 ` Dave Chinner
2010-10-17 2:25 ` Nick Piggin
2010-10-18 16:16 ` Andi Kleen
2010-10-18 16:21 ` Christoph Hellwig
2010-10-19 7:00 ` Nick Piggin
2010-10-19 16:50 ` Christoph Hellwig
2010-10-20 3:11 ` Nick Piggin
2010-10-24 15:44 ` Thomas Gleixner
2010-10-24 21:17 ` Nick Piggin
2010-10-25 4:41 ` Thomas Gleixner
2010-10-25 7:04 ` Thomas Gleixner
2010-10-26 0:12 ` Nick Piggin
2010-10-26 0:06 ` Nick Piggin
2010-10-08 5:21 ` [PATCH 12/18] fs: add a per-superblock lock for the inode list Dave Chinner
2010-10-08 7:35 ` Christoph Hellwig
2010-10-08 5:21 ` [PATCH 13/18] fs: split locking of inode writeback and LRU lists Dave Chinner
2010-10-08 7:42 ` Christoph Hellwig
2010-10-08 8:00 ` Dave Chinner
2010-10-08 8:18 ` Christoph Hellwig
2010-10-16 7:57 ` Nick Piggin
2010-10-16 16:20 ` Christoph Hellwig
2010-10-16 17:19 ` Nick Piggin
2010-10-17 1:00 ` Dave Chinner
2010-10-17 2:20 ` Nick Piggin
2010-10-08 5:21 ` [PATCH 14/18] fs: Protect inode->i_state with th einode->i_lock Dave Chinner
2010-10-08 7:49 ` Christoph Hellwig
2010-10-08 8:04 ` Dave Chinner
2010-10-08 8:18 ` Christoph Hellwig
2010-10-16 7:57 ` Nick Piggin
2010-10-16 16:19 ` Christoph Hellwig
2010-10-09 8:05 ` Christoph Hellwig
2010-10-09 14:52 ` Matthew Wilcox
2010-10-10 2:01 ` Dave Chinner
2010-10-08 5:21 ` [PATCH 15/18] fs: introduce a per-cpu last_ino allocator Dave Chinner
2010-10-08 7:53 ` Christoph Hellwig
2010-10-08 8:05 ` Dave Chinner
2010-10-08 8:22 ` Andi Kleen
2010-10-08 8:44 ` Christoph Hellwig
2010-10-08 9:58 ` Al Viro
2010-10-08 10:09 ` Andi Kleen
2010-10-08 10:19 ` Al Viro
2010-10-08 10:20 ` Eric Dumazet
2010-10-08 9:56 ` Al Viro
2010-10-08 10:03 ` Christoph Hellwig
2010-10-08 10:20 ` Eric Dumazet
2010-10-08 13:48 ` Christoph Hellwig
2010-10-08 14:06 ` Eric Dumazet
2010-10-08 19:10 ` Christoph Hellwig
2010-10-09 17:14 ` Matthew Wilcox
2010-10-16 7:57 ` Nick Piggin
2010-10-16 16:22 ` Christoph Hellwig
2010-10-16 17:21 ` Nick Piggin
2010-10-08 5:21 ` [PATCH 16/18] fs: Make iunique independent of inode_lock Dave Chinner
2010-10-08 7:55 ` Christoph Hellwig
2010-10-08 8:06 ` Dave Chinner
2010-10-08 8:19 ` Christoph Hellwig
2010-10-08 5:21 ` [PATCH 17/18] fs: icache remove inode_lock Dave Chinner
2010-10-08 8:03 ` Christoph Hellwig
2010-10-08 8:09 ` Dave Chinner
2010-10-13 7:20 ` Nick Piggin
2010-10-13 7:27 ` Nick Piggin
2010-10-13 11:28 ` Christoph Hellwig
2010-10-13 12:03 ` Nick Piggin
2010-10-13 12:20 ` Christoph Hellwig
2010-10-13 12:25 ` Nick Piggin
2010-10-13 10:42 ` Eric Dumazet
2010-10-13 12:07 ` Nick Piggin
2010-10-13 11:25 ` Christoph Hellwig
2010-10-13 12:30 ` Nick Piggin
2010-10-13 23:23 ` Dave Chinner
2010-10-14 9:06 ` Nick Piggin
2010-10-14 9:13 ` Nick Piggin
2010-10-14 14:41 ` Christoph Hellwig
2010-10-15 0:14 ` Nick Piggin
2010-10-15 3:13 ` Dave Chinner
2010-10-15 3:30 ` Nick Piggin
2010-10-15 3:44 ` Nick Piggin
2010-10-15 6:41 ` Nick Piggin [this message]
2010-10-15 10:59 ` Dave Chinner
2010-10-15 13:03 ` Nick Piggin
2010-10-15 13:29 ` Nick Piggin
2010-10-15 17:33 ` Nick Piggin
2010-10-15 17:52 ` Christoph Hellwig
2010-10-15 18:02 ` Nick Piggin
2010-10-15 18:14 ` Nick Piggin
2010-10-16 2:09 ` Nick Piggin
2010-10-15 14:11 ` Nick Piggin
2010-10-15 20:50 ` Nick Piggin
2010-10-15 20:56 ` Nick Piggin
2010-10-15 4:04 ` Nick Piggin
2010-10-15 11:33 ` Dave Chinner
2010-10-15 13:14 ` Nick Piggin
2010-10-15 15:38 ` Nick Piggin
2010-10-16 7:57 ` Nick Piggin
2010-10-08 5:21 ` [PATCH 18/18] fs: Reduce inode I_FREEING and factor inode disposal Dave Chinner
2010-10-08 8:11 ` Christoph Hellwig
2010-10-08 10:18 ` Al Viro
2010-10-08 10:52 ` Dave Chinner
2010-10-08 12:10 ` Al Viro
2010-10-08 13:55 ` Dave Chinner
2010-10-09 17:22 ` Matthew Wilcox
2010-10-09 8:08 ` [PATCH 19/18] fs: split __inode_add_to_list Christoph Hellwig
2010-10-12 10:47 ` Dave Chinner
2010-10-12 11:31 ` Christoph Hellwig
2010-10-12 12:05 ` Dave Chinner
2010-10-09 11:18 ` [PATCH 20/18] fs: do not assign default i_ino in new_inode Christoph Hellwig
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=20101015064150.GB7511@amd \
--to=npiggin@kernel.dk \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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 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).