All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Linux 2.6.35
Date: Mon, 2 Aug 2010 15:58:34 +1000	[thread overview]
Message-ID: <20100802055834.GB19164@dastard> (raw)
In-Reply-To: <AANLkTint_s6h_EC7mDiPsyxr=C0GSrnrgJYkCCU7JEtN@mail.gmail.com>

On Sun, Aug 01, 2010 at 07:50:02PM -0700, Linus Torvalds wrote:
> On Sun, Aug 1, 2010 at 7:33 PM, Dave Chinner <david@fromorbit.com> wrote:
> >
> > There hasn't been nearly enough review or testing of this patch
> > series yet.  Before a merge, it needs to be split up in smaller,
> > more digestable chunks for more comprehensive review, regression
> > testing and behavioural analysis.
> 
> I dunno. We merge _way_ scarier things in the VM and the block layer,
> for much less actual upside, and with less review.

Scary stuff outside of direct VFS/FS interfaces is generally hidden
from me by my +6 Blinkers of Blissful Ignorance. I make the
assumption that the experts involved know the risks and have weighed
them up appropriately. ;)

> The RCU pathname lookup has some rather impressive performance
> upsides, and I agree that it would be good to get a lot of review and
> testing, but the latter isn't going to happen without it being
> mainlined, and the former is sadly lacking. The person I'd like most
> to review it is Al,

Most definitely.

> but anybody in the filesystem world should
> basically see it as a #1 priority,

Agreed - I've actually looked at every patch, commented on some
of the more questionable things, got quoted by LWN for saying that
it "fell off the locking cliff", have run benchmarks on it and sent
patches fixing bugs back to Nick.

It's just really hard to digest it all in one lump and core VFS
changes on this scale scare me....

> because unlike all the masturbatory
> patches like xstat() that add new functionality that nobody will
> likely ever use, Nick's patchseries improves on the thing that
> everybody uses heavily every day without even thinking about it.
> 
> Is it tough to review? Yes. It's core code, not just some random
> addition that adds a new feature and doesn't impact any old code. But
> that's also the thing that makes it meaningful, and makes me think it
> should get merged _much_ more eagerly than most code we ever see.

I agree with you for the pure locking changes.

But for the bits that change writeback, LRU ordering and reclaim
calculations the benefits are not quite so obvious, nor is the
correctness of the code/behaviour quite so provably correct.  Maybe
I'm being a bit too paranoid, but generally it pays to be a bit
conservative as a filesystem developer because the cost of screwing
up can be pretty high...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2010-08-02  5:58 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-01 23:52 Linux 2.6.35 Linus Torvalds
2010-08-02  0:32 ` Stephen Rothwell
2010-08-02  8:14   ` Nick Piggin
2010-08-02  8:52     ` Stephen Rothwell
2010-08-02  2:33 ` Dave Chinner
2010-08-02  2:50   ` Linus Torvalds
2010-08-02  5:58     ` Dave Chinner [this message]
2010-08-02  7:55       ` Nick Piggin
2010-08-02  7:55         ` Nick Piggin
2010-08-02  8:24         ` Christoph Hellwig
2010-08-02  8:46           ` KOSAKI Motohiro
2010-08-02  9:05           ` Christoph Hellwig
2010-08-02 10:07             ` Nick Piggin
2010-08-02  9:51           ` Nick Piggin
2010-08-03  8:18   ` Andi Kleen
2010-08-03  9:28     ` Nick Piggin
2010-08-03  9:49       ` Andi Kleen
2010-08-03 15:05       ` Henrique de Moraes Holschuh
2010-08-02  8:51 ` make 3.82 fails on powerpc defconfig update [was: Linux 2.6.35] Thomas Backlund
2010-08-02  8:51   ` Thomas Backlund
2010-08-02 18:28   ` Sam Ravnborg
2010-08-02 18:28     ` Sam Ravnborg
2010-08-02 20:46     ` Thomas Backlund
2010-08-02 20:51       ` Sam Ravnborg
2010-08-02 21:02         ` Andreas Schwab
2010-08-02 21:02           ` Andreas Schwab
2010-08-02 21:03         ` Thomas Backlund
2010-08-02 21:03           ` Thomas Backlund
2010-08-03  6:48           ` Sam Ravnborg
2010-08-07 17:56   ` Paul Smith
2010-08-07 17:56     ` Paul Smith
  -- strict thread matches above, loose matches on Subject: below --
2010-08-02  2:31 Linux 2.6.35 Donald Parsons
2010-08-02  3:28 ` Randy Dunlap
2010-08-02  3:38 ` Bjorn Helgaas
2010-08-02  4:21   ` Donald Parsons
2010-08-02 13:48     ` Bjorn Helgaas
2010-08-02 15:59       ` Harald Hoyer
2010-08-02 22:09         ` Frédéric L. W. Meunier
2010-08-02 22:34           ` Frédéric L. W. Meunier
2010-08-02 18:38       ` Donald Parsons
2010-08-02 16:08     ` Harald Hoyer
2010-08-03  2:31       ` Donald Parsons
2010-08-03  4:42         ` Randy Dunlap
2010-08-03 10:24           ` Stefan Richter
2010-08-03 18:26             ` Donald Parsons
2010-08-03 16:26           ` Donald Parsons
2010-08-03 16:40             ` Randy Dunlap
2010-08-03 17:45               ` Donald Parsons
2010-08-03 18:35                 ` Randy Dunlap
2010-08-03 22:34                 ` Randy Dunlap
2010-08-04 20:15         ` Jeff Garzik

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=20100802055834.GB19164@dastard \
    --to=david@fromorbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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.