Re: [PATCH, 3.7-rc7, RESEND] fs: revert commit bbdd6808 to fallocate UAPI

Chris Mason
2012-12-07T19:03:09Z

On Fri, Dec 07, 2012 at 11:18:00AM -0700, Linus Torvalds wrote:
>
>
> On Fri, 7 Dec 2012, Ric Wheeler wrote:
> >
> > Review is part of the way we work as a community and we should figure out how
> > to fix our review process so that we can have meaningful results from the
> > review or we lose confidence in the process and it makes it much harder to get
> > reviewers to spend time reviewing when their reviews are ultimately ignored.
>
> Christ, I promised myself to not respond any more to this thread, but the
> insanity just continues, from people who damn well should know better.
>
> The code wasn't merged. The review worked.
>
> What you (and Dave, and Christoph) are trying to do is shut down a feature
> that somebody else decided they needed. That's not what code review is all
> about, and dammit, don't try to even claim it is.
>
> So stop these dishonest and disingenious arguments. They are full of crap.
>
> No amount of "review" has any meaning what-so-ever on whether somebody
> else decides they need a feature or not. You can review all you want, but
> it's irrelevant - if some company decides they are going to ship or use a
> feature, it's out of your hands.
>
> What got merged was a ONE-LINER to make sure that possible future
> development didn't unnecessarily make things any more confusing, with the
> knowledge that there was a user of the code you didn't like.
>
> Every single argument I've heard of from the "please revert" camp has been
> inane. And they've been *transparently* inane, to the point where I don't
> understand how you can make them with a straght face and not be ashamed.

I really agree with Dave's statement that we should ioctl for private
features and system call for features other filesystems are likely to
implement. So we really shouldn't have private bits in fallocate in use
in production systems.

That's not what happened though, and the right way forward from here is
to give the bit to the feature, maybe with a generic name like
FALLOCATE_WITHOUT_BEING_HORRIBLY_SLOW. It should have been done
differently, but it wasn't. And it's a problem we all have, so it makes
sense that we'll all want to address it somehow.

On a single flash drive doing random 4K writes, xfs does 950MB/s into
regular extents but only 400MB/s into preallocated extents.

http://masoncoding.com/presentation/perf-linuxcon12/fallocate.png

ext4 has a bigger hit, but there's a little room for improvement all
around.

Maybe we should use this thread as the starting point for the proper
12-18 months of bike shedding for a real fix?

-chris