23 captures

19 Dec 2012 - 18 Dec 2025

Apr	MAY	Jun
	24
2012	2013	2014

success

fail

About this capture

COLLECTED BY

Organization: Internet Archive

The Internet Archive discovers and captures web pages through many different web crawls. At any given time several distinct crawls are running, some for months, and some every day or longer. View the web archive through the Wayback Machine.

Collection: Wide Crawl started April 2013

Web wide crawl with initial seedlist and crawler configuration from April 2013.

TIMESTAMPS

The Wayback Machine - http://web.archive.org/web/20130524085018/http://lwn.net/Articles/528107/

Create an account

Subscribe to LWN

Return to the Kernel page

LWN.net Weekly Edition for May 23, 2013

An "enum" for Python 3

An unexpected perf feature

LWN.net Weekly Edition for May 16, 2013

A look at the PyPy 2.0 release

Weekly edition	Kernel	Security	Distributions	Contact Us	Search
Archives	Calendar	Subscribe	Write for LWN	LWN.net FAQ	Sponsors

A FALLOC_FL_NO_HIDE_STALE followup

ByJonathan Corbet
December 5, 2012

Last week's edition included an article on the addition of the FALLOC_FL_NO_HIDE_STALE flag to the fallocate() system call. Some developers, objecting to the patch and the way it got into the kernel, had called for it to be reverted before the 3.7 release went final. At the time, Linus had not made any remarks in the discussion or indicated whether he would accept the revert.

That situation changed after Linus was prompted by Martin Steigerwald. His response was clear enough:

If you want something reverted, you show me the *technical* reason for it. Not the "ooh, I'm so annoyed by how this was done" reason for it.

And if your little feelings got hurt, get your mommy to tuck you in, don't email me about it. Because I'm not exactly known for my deep emotional understanding and supportive personality, am I?

There were some technical reasons offered in the discussion, along with the more general process-oriented complaints. But it seems clear that Linus has not found that discussion convincing. So, in the absence of a surprise from somewhere, it seems that the new fallocate() flag will remain for the 3.7 release, at which point it will become part of the kernel's user-space ABI.

(Log in to post comments)

A FALLOC_FL_NO_HIDE_STALE followup

Posted Dec 6, 2012 6:55 UTC (Thu) by bergwolf (subscriber, #55931) [Link]

FALLOC_FL_NO_HIDE_STALE has potential security issues. Why do people think it is OK to implement such thing in the generic file system interface?

A FALLOC_FL_NO_HIDE_STALE followup

Posted Dec 6, 2012 13:06 UTC (Thu) by andresfreund (subscriber, #69562) [Link]

Well, as the previous article noted, they propose that a mount option restricts usage of that facility to a specific group. So its not like it gives away anything by default.

Afaics the only thing that happened for 3.7 was the introduction of that flag, nothing uses it yet, so there cannot be said too much about the security implications of the real thing...

A FALLOC_FL_NO_HIDE_STALE followup

Posted Dec 6, 2012 20:45 UTC (Thu) by cesarb (subscriber, #6266) [Link]

I can see one security implication: inexperienced programmers who hear about the flag and use it because "it makes things go faster", and get away with it because the flag does nothing. Then their program is deployed somewhere which actually uses the flag, and their code breaks because it assumed zero-fill.

It would be better if any attempt to use the flag always returned -EPERM.

A FALLOC_FL_NO_HIDE_STALE followup

Posted Dec 6, 2012 20:57 UTC (Thu) by andresfreund (subscriber, #69562) [Link]

There's no user of the flag in the kernel yet, but the proposed patch did exactly that:

int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
{
@@ -249,6 +254,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
if (ret)
return ret;

+ /* Check for enabling _NO_HIDE_STALE flag */
+ if (mode & FALLOC_FL_NO_HIDE_STALE &&
+ !sysctl_enable_falloc_no_hide_stale)
+ return -EPERM;
+

So such inexperienced programmers would fall on their noses.

I don't think the process in which this got through was great, but why assume the people working on this are stupid?

A FALLOC_FL_NO_HIDE_STALE followup

Posted Dec 6, 2012 21:43 UTC (Thu) by cesarb (subscriber, #6266) [Link]

I just took a look at the current kernel, and the problem I imagined does not exist. The kernel already returns an error if FALLOC_FL_NO_HIDE_STALE is passed to sys_fallocate:

/* Return error if mode is not supported */
if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
return -EOPNOTSUPP;

So any inexperienced programmer incorrectly attempting to use the flag to "make things go faster" will already receive an error, and the fallocate call will do nothing.

Gotta love "notwithstanding"

Posted Dec 7, 2012 13:12 UTC (Fri) by man_ls (subscriber, #15091) [Link]

I thought that the technical reason was that the flag was useful only for one filesystem (ext4), and only because it had bad performance when returning unallocated blocks because it had to zero them first. In the original article it was claimed that the right solution for this problem was to fix ext4, not hide the problem with a flag. Looks like sound technical reasons to me, Linus trash talk notwithstanding.

There is also something slightly annoying when a process gets subverted by a maintainer just because the company paying his bills needs it. No technical excellence to be seen anywhere. Perhaps my ignorance about this problem is even deeper than for kernel development in general, kind of a Marianas Trench of knowledge.

Gotta love "notwithstanding"

Posted Dec 7, 2012 19:44 UTC (Fri) by lacos (subscriber, #70616) [Link]

Right. The burden of proof ("show me the *technical* reason") is on whomever wants to get the patch *in*. The default is "no change". Ad-hoc reversal of the burden of proof undermines the credibility of the entire process. The last paragraph in <https://lwn.net/Articles/527390/> says it all. Linus' reaction is blatantly partial.

Gotta love "notwithstanding"

Posted Dec 7, 2012 20:21 UTC (Fri) by neilbrown (subscriber, #359) [Link]

I admit that it could look that way, but I think the reality is that Linus is being blatantly pragmatic.
The harm caused by the patch is not technical, and may even be purely theoretical, it is hard to measure.
The benefit of the patch is that it makes it easier and safer for someone out there to use Linux, and that is a genuine positive.

The only real negative is that it seems that Ted slipped the patch in "under the radar", arguably misusing his status as a maintainer. They might hurt his reputation a little, but that isn't Linus' problem, and Ted has a very strong reputation so I don't think a little hurt will make a big dent.

You might argue that there is a risk that other maintainers might think that Ted got away with something and might try the same thing. I agree that is possible but I suspect it is fairly unlikely and I very much doubt they would get away with it.

But Linus' pragmatic approach is a lot more about solving problems that are, not solving problems that might be, and his decision is consistent with that approach.

Our actions of today

Posted Dec 7, 2012 23:24 UTC (Fri) by man_ls (subscriber, #15091) [Link]

The problem I see is that the next time that Nvidia or some other out-of-tree proprietary developer asks for some flag to be reserved for their own use, they can point to FALLOC_FL_NO_HIDE_STALE and say that they just want the same harmless favor: a teeny tiny flag to be reserved so that no one stomps over their nice proprietary code that solves their own situation and is of no use (or actively harmful) to anyone else, so that "it is easier and safer for someone out there to use Linux".

Probably Linus will say "get lost" and it will not matter a bit, so perhaps there is really no problem.

Our actions of today

Posted Dec 8, 2012 1:36 UTC (Sat) by Cyberax (✭ supporter ✭, #52523) [Link]

Google's patch is GPL-ed, but not mainlined. Linus is pragmatic and inconsistent whenever it suits him.

Only the flag. Not the code.

Posted Dec 7, 2012 21:51 UTC (Fri) by zlynx (subscriber, #2285) [Link]

All that got in is the flag. None of the code that implements the flag is in the kernel. This is just in so that no one else ever uses that flag value.

Some people seem to be arguing against this patch as if it was the implementation code. Stop.

The flag is the code

Posted Dec 7, 2012 23:19 UTC (Fri) by man_ls (subscriber, #15091) [Link]

What do you mean exactly? Frankly I am lost. Flags don't need a technical reason to be reserved? Or that reserving flags can be done by maintainers without discussion?

A flag is a name for a bit position. Not code.

Posted Dec 8, 2012 3:39 UTC (Sat) by zlynx (subscriber, #2285) [Link]

I mean it is a flag in a field that isn't short of flags. No one else wants that flag. It hurts no one to use that bit.

So there is no valid technical reason to exclude the flag and there IS a good reason to include it, just as syscall, ioctl and device numbers have been reserved in the past. So that new feature patches don't get created that use the same bit. So that future authors writing out of tree code don't end up with a user-space that has to be recompiled when they merge other kernel features.

Objecting to the "process violation" is one thing, and perhaps deserves a post or two. But what then? For ignoring the bureaucracy it seems people want to administer "punishment" by not including the flag. Which is ridiculous at this point because by NOW there HAS been a lot of discussion. Linus has reviewed the patch and accepted it. So the discussion has all been done and the "process" has been successfully followed.

And FYI, code gets changed by maintainers without discussion all the time. Whenever there's a merge conflict between trees is one example.

A flag is a name for a bit position. Not code.

Posted Dec 9, 2012 21:54 UTC (Sun) by nevets (subscriber, #11875) [Link]

> And FYI, code gets changed by maintainers without discussion all the time. Whenever there's a merge conflict between trees is one example.

Except this was a controversial change that was NACKed previously. Slipping trivial changes and merge conflict resolutions in without review is one thing. Slipping a change that has been previously NACKed by other maintainers is something completely different.

A flag is a name for a bit position. Not code.

Posted Dec 9, 2012 22:09 UTC (Sun) by zlynx (subscriber, #2285) [Link]

That is why the change that was NACK'd wasn't merged. That would have been the feature and all its code. The flag reservation is what, 1 line, and really not worth all of this noise.

A flag is a name for a bit position. Not code.

Posted Dec 10, 2012 0:56 UTC (Mon) by nevets (subscriber, #11875) [Link]

The change itself was obviously still controversial. It's reserving a bit to a system call that affects all filesystems, and it was pushed in by one filesystem maintainer because his company needed it for out of tree code that was NACKed.

It still should have been posted for comments. Even if it suffered more NACKs, Ted could have posted it to Linus and stated that this is the best solution so far, and it's currently in use by Google. Linus currently seems to be fine with the change, and could have pulled it regardless of the NACKs by other maintainers. He's done things like that before. If that had happened, the other maintainers may have their feelings hurt, but at least everything was out in the open and honest.

I go back to my original statement. It may be a one-liner, but its against a system call and for out of tree code that has been NACKed before. It's not a trivial fix nor a merge conflict. It should have been discussed.

Only the flag. Not the code.

Posted Dec 8, 2012 13:43 UTC (Sat) by ricwheeler (subscriber, #4980) [Link]

The bigger issue is not the flag. The core is that this "feature" is not even used by its advocates - exposing unwritten, stale data.

The only justification for the feature or the flag is to take advantage of the side effect which makes ext4 go faster.

Note that on the ext4 list we did debate multiple techniques to fix the actual performance problem at the core here (whether or not random, single fs block writes to a preallocated file are a critical use case).

The problem with the flag itself is that people will use google, etc to search for "ext4 is slow" and suddenly come across the maintainer talking about how to make it faster by using this feature without proper context. Even the name of the flag is misleading - "no hide" sounds like we are doing something fishy.

Better to use a name like "show me other peoples data to make my workload go faster" :)

Gotta love "notwithstanding"

Posted Dec 11, 2012 12:14 UTC (Tue) by dgm (subscriber, #49227) [Link]

Which, if you think about it, is a great way to avoid to become an slave of the process itself. Processes should not be immutable law. As all sets of rules, they are made to be broken when it's reasonable.

Rule number one should be "use your brain". Mindlessly following rules is for bureaucrats and children under five.

Copyright © 2012, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds