linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Piggin <npiggin@kernel.dk>
To: Christoph Hellwig <hch@infradead.org>
Cc: Nick Piggin <npiggin@kernel.dk>,
	Dave Chinner <david@fromorbit.com>,
	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 15:04:06 +1100	[thread overview]
Message-ID: <20101015040406.GA6930@amd> (raw)
In-Reply-To: <20101014144159.GA11972@infradead.org>

On Thu, Oct 14, 2010 at 10:41:59AM -0400, Christoph Hellwig wrote:
> > Things
> > like path walks are nearly 50% faster single threaded, and perfectly
> > scalable. Linus actually wants the store-free path walk stuff
> > _before_ any of the other things, if that gives you an idea of where
> > other people are putting the priority of the patches.
> 
> Different people have different priorities.  In the end the person
> doing the work of actually getting it in a mergeable shape is setting
> the pace.  If you had started splitting out the RCU pathwalk bits half a
> year ago there's we already have it in now.  But that's now how it
> worked.

Also, I appreciate that you have run into some lock scaling on these XFS
workloads. But the right way to do it is not to quickly solve your own
issues and then sort out the rest, leaving my tree in wreckage.

Yes it will take a bit longer to actually solve *everyone*'s problems
(that is most definitely including NUMA reclaim, and path walk ping pong
and performance). But we can do it in a coherent way and we can do so
looking at and testing the _end_ result.

Once everyone is happy with where we want to go, it is a matter of
making nice mergable pieces, I agree my patchset still has some work to
do here. But this process is not going to "drag out", by doing it this
way. It can probably be done in 2 releases (first inode, then dcache).
In fact on the contrary I think it is much better to get the whole
group of changes merged at once.

If it drags out and we sqabble and don't agree on what locking is
required _before_ we start merging things, then it will end up with a
half finished mess of slowly changing locking over many kernel releases.

So rather than taking a few bits that you particularly want solved right
now and not bothering to look at the rest because you claim they're
weird or not needed or controversial is really not helping the way I
want to merge this.

And really, blaming me for a few weeks vacation and a few other weeks
on work related stuff for causing all these delays is ridiculous. I've
been posting bits and pieces and ideas and rfcs for a long time without
any real interest from vfs people at all.  The only real time I heard
from you about anything is a couple of times when I actually posted the
full patchset, you'd whinge about it was unreviewable (disregarding that
it was split into individually reviewable pieces and provided an overall
view of where I was going).

_I_ have actually been talking to people, running tests on big machines,
working with -rt guys, socket/networking people, etc. I've worked
through the store-free lookup design with Linus, we've agreed on RCU
inodes and contingency to manage unexpected regressions.

So when you just handwave away "little problems" like proper per-zone
reclaim, rcu-walk path lookup, or scaling the hash manipulations as
"controversial, weird, I'm not sold" it's really frustrating. Especially
when you turn around and accuse me of continually delaying things in the
same email.

  parent reply	other threads:[~2010-10-15  4:04 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
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 [this message]
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=20101015040406.GA6930@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).