linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Martin Steigerwald <Martin@lichtvoll.de>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Theodore Ts'o <tytso@mit.edu>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH, 3.7-rc7, RESEND] fs: revert commit bbdd6808 to fallocate UAPI
Date: Fri, 7 Dec 2012 13:40:49 +1100	[thread overview]
Message-ID: <20121207024049.GF27172@dastard> (raw)
In-Reply-To: <20121207010837.GA16373@gmail.com>

On Fri, Dec 07, 2012 at 02:08:37AM +0100, Ingo Molnar wrote:
> 
> * Martin Steigerwald <Martin@lichtvoll.de> wrote:
> 
> > > The thing that people are complaining about is exactly the 
> > > reverse of this. It's *protecting* us from making mistakes, 
> > > and doesn't actually add any new interfaces in itself.
> > >
> > > This is why I'm so annoyed with this stupid thread. It's 
> > > been going on forever, and reverting that change WOULD BE 
> > > OBJECTIVELY A BAD IDEA.
> > 
> > See, thats where you have a problem with "reality".
> > 
> > It seems you cannot accept the fact that some developers 
> > disliked the process in which this change was pushed. [...]
> 
> I don't think you have understood Linus's argument above.
> 
> The "process" does not change the object technical merits of a 
> patch. Ever. This patch is _good_, and objectively good. No 
> amount of 'bad process' can make this patch bad.

Yeah, Linus asserted the patch is good, and you're parroting that.
There isn't even any agreement on that level, let alone the other
problems like the fact it introduced undocumented changes to a
syscall API. people get raked over the coals frequently for doing
this sort of stuff, but in this case it's "good because Linus said
so".

Review gives the opportunity to make patches *better*. It might be
a good concept with a bad implementation (which is how I'd
categorise this patch), and review gives us the opportunity to sort
out those problems before we end up in a situation like this.

Especially for changes to syscall APIs....

> Now, hypothetically, if this was an objectively bad patch, then 
> any "bad process" used to push it would add insult to injury and 
> it could be reason enough to flame Tytso twice as hard.

Assumption: the patch is good

Reality: the concept is worthy, but the implmentation and and the
taken were terrible. Patch could be a lot better, but improvement
needs time and for syscall changes already in mainline we don't have
that time. either we revert it or we are stuck with it.

Right now, we're stuck with it....

> But it turns out the patch was right and good, so kudos to Tytso 
> for cutting through the bike shed painting and politicks of 
> fsdevel - which "process" would have deprived us of a good 
> patch...

Assumption: a) it's a good patch and b) review would have prevented
the patch from proceeding.

Reality: a) see above. b) Who can say - it never occurred?
Personally, I'm not opposed to reserving a bit but it needs
documentation and clarification over future scope and kernel
implementation, which review would have caught and the discussion
would have been in that context....

If you can't stand the heat, get out of the kitchen. IOWs, if you
can't defend your change on it's merits, then it shouldn't be made.
Sending your patch through a back door becuse you don't think you
can't defend it as you pass through the front door is simply *not
acceptable*.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2012-12-07  2:40 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-19 23:04 [PATCH] fs: revert commit bbdd6808 to fallocate UAPI Dave Chinner
2012-11-20 16:36 ` Christoph Hellwig
2012-11-26  0:28 ` [PATCH, 3.7-rc7, RESEND] " Dave Chinner
2012-11-26  2:55   ` Theodore Ts'o
2012-11-26  6:14     ` Tao Ma
2012-11-26  9:12     ` Dave Chinner
2012-12-05 10:48       ` Martin Steigerwald
2012-12-05 15:45         ` Linus Torvalds
2012-12-05 16:18           ` Martin Steigerwald
2012-12-05 16:33             ` Theodore Ts'o
2012-12-05 17:24               ` Martin Steigerwald
2012-12-05 17:34                 ` Theodore Ts'o
2012-12-05 17:55                   ` Martin Steigerwald
2012-12-06  0:42                   ` Dave Chinner
2012-12-06  9:24                     ` Martin Steigerwald
2012-12-05 18:25             ` Linus Torvalds
2012-12-06  1:14               ` Dave Chinner
2012-12-06  3:03                 ` Linus Torvalds
2012-12-06  9:37                   ` Martin Steigerwald
2012-12-07  1:08                     ` Ingo Molnar
2012-12-07  2:40                       ` Dave Chinner [this message]
2012-12-07 10:24                       ` Martin Steigerwald
2012-12-06 12:06                 ` Christoph Hellwig
2012-12-06 16:50                   ` Theodore Ts'o
2012-12-07  1:57                     ` Dave Chinner
2012-12-06 12:05           ` Christoph Hellwig
2012-12-07  1:16             ` Ingo Molnar
2012-12-07  3:19               ` Dave Chinner
2012-12-07 17:36               ` Ric Wheeler
2012-12-07 18:18                 ` Linus Torvalds
2012-12-07 19:03                   ` Chris Mason
2012-12-07 20:43                     ` Theodore Ts'o
2012-12-07 21:09                       ` Chris Mason
2012-12-07 21:27                         ` Theodore Ts'o
2012-12-07 21:43                           ` Chris Mason
2012-12-07 21:49                             ` Ric Wheeler
2012-12-07 21:57                               ` Chris Mason
2012-12-07 22:51                                 ` Eric Sandeen
2012-12-07 22:52                                 ` Eric Sandeen
2012-12-07 21:42                         ` Ric Wheeler
2012-12-07 21:57                           ` Theodore Ts'o
2012-12-07 22:02                             ` Ric Wheeler
2012-12-08  0:39                               ` Dave Chinner
2012-12-08  2:52                                 ` Joel Becker
2012-12-08  4:04                                   ` Dave Chinner
2012-12-08  0:17                     ` Dave Chinner
2012-12-08  1:39                       ` Chris Mason
2012-12-10 16:02                         ` Chris Mason
2012-12-10 17:37                       ` Theodore Ts'o
2012-12-10 18:05                         ` Steven Whitehouse
2012-12-10 18:13                           ` Theodore Ts'o
2012-12-10 18:20                             ` Theodore Ts'o
2012-12-11 12:16                               ` Steven Whitehouse
2012-12-11 22:09                                 ` Dave Chinner
2012-12-10 18:52                         ` Ric Wheeler
2012-12-11  0:52                         ` Dave Chinner
2012-12-07 19:30                   ` Steven Rostedt
2012-12-07 21:14                     ` Theodore Ts'o
2012-12-07 21:47                       ` Ric Wheeler
2012-12-07 23:25                         ` Howard Chu
2012-12-08  0:50                           ` Dave Chinner
2012-12-08 13:52                             ` Howard Chu
2012-12-08 14:02                               ` Ric Wheeler
2012-12-07 22:01                       ` Eric Sandeen
2012-12-09 21:37                       ` Ric Wheeler
2012-11-26 11:53     ` Alan Cox
2012-11-26 14:43       ` Theodore Ts'o
2012-11-26 21:12       ` Dave Chinner
2012-11-27 13:44         ` Martin Steigerwald

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=20121207024049.GF27172@dastard \
    --to=david@fromorbit.com \
    --cc=Martin@lichtvoll.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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).