lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20121207024049.GF27172@dastard>
Date:	Fri, 7 Dec 2012 13:40:49 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Martin Steigerwald <Martin@...htvoll.de>,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Theodore Ts'o <tytso@....edu>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH, 3.7-rc7, RESEND] fs: revert commit bbdd6808 to fallocate
 UAPI

On Fri, Dec 07, 2012 at 02:08:37AM +0100, Ingo Molnar wrote:
> 
> * Martin Steigerwald <Martin@...htvoll.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@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ