Posted Oct 29, 2012 21:27 UTC (Mon) by bronson (subscriber, #4806)
Parent article: Thoughts on the ext4 panic
Nice job Nix. Your tenacity has made the world a better place. Not necessarily because many people were going to lose data to this bug, but because you showed how something that looks like an obviously simple and safe feature can cut in surprising ways.
Posted Oct 29, 2012 22:36 UTC (Mon) by nix (subscriber, #2304)
[Link]
Oh yes. For why async commits require checksumming, see fs/jbd2/recovery.c:do_one_pass(), the switch case for JBD2_COMMIT_BLOCK.
As an aside, this function provides a fairly strong argument against one of eight-character tab indentation and/or wrapping at 80 characters... the lines are so squashed and short that it's really terribly hard to read. Personally I'm more in favour of a '70 character lines, disregarding indentation' rule, which prevents lines becoming too long to be readable without constraining indentation depth hugely: in do_one_pass(), the effective line length due to one while loop plus a switch statement plus a loop or conditional inside one of the cases is about thirty characters, which is just ridiculous. (As for pandering to the tiny minority of people still writing code on VT102s, they can go and get some hardware built sometime in the last twenty years. Punched cards are obsolete: so are 80-character terminals, IMNSHO.)
Thoughts on the ext4 panic
Posted Oct 30, 2012 0:55 UTC (Tue) by ikm (subscriber, #493)
[Link]
When one runs a terminal emulator, it's usually 80-chars wide by default. I have a strong feeling that the 80-chars wrapping is still considered relevant largely because of that default.
Thoughts on the ext4 panic
Posted Oct 30, 2012 4:28 UTC (Tue) by martinfick (subscriber, #4455)
[Link]
Or perhaps enough programmers still like to put more than one terminal or editor on their screen at once side by side? I suspect that who use IDEs forget that this is even possible.
Thoughts on the ext4 panic
Posted Oct 30, 2012 7:14 UTC (Tue) by Aliasundercover (subscriber, #69009)
[Link]
What is it with the IDEs not allowing multiple source windows? I can't imagine working without my screen filled with various views of my source. The IDEs I look at give tabs but no way to display multiple at once. When writing code I normally want to see a header defining the structures I'm using, some other source file which my code interacts with, yet another source file where I did something vaguely similar once before ...
I don't see a Linux IDE which allows this. Visual Studio on Windows does, I just have to tell it to work in its multiple document mode.
If given the choice between a pack of x-terms and vi straight out of the 80s and a modern IDE with all the bells and whistles but just one source window I would take x-terms and vi. Hard to imagine how anyone can use these IDEs much less design them the way they are.
Thoughts on the ext4 panic
Posted Oct 30, 2012 12:42 UTC (Tue) by fb (subscriber, #53265)
[Link]
> What is it with the IDEs not allowing multiple source windows? [...]
> I don't see a Linux IDE which allows this.
FYI, just checked and Eclipse (at least Juno/4.2) allows this. I can drag a source tab and place it next to my main source view.
Thoughts on the ext4 panic
Posted Oct 30, 2012 17:26 UTC (Tue) by marcH (subscriber, #57642)
[Link]
It is indeed much easier to split and generally manage the screen with a full screen Eclipse than with most window managers.
Thoughts on the ext4 panic
Posted Oct 30, 2012 21:17 UTC (Tue) by khc (subscriber, #45209)
[Link]
But only if all the windows you need to manage are eclipse windows, of course. I recently started to play with Dart using the Dart Editor which is basically eclipse, it works ok, but I don't know if I'd consider managing a full screen eclipse along with other windows "easy"
Thoughts on the ext4 panic
Posted Oct 31, 2012 8:40 UTC (Wed) by marcH (subscriber, #57642)
[Link]
> but I don't know if I'd consider managing a full screen eclipse along with other windows "easy"
No it's not easy; you are not supposed to do that. Instead, you are supposed to install and use whatever plug-ins give you the functionality you need *within* Eclipse.
Eclipse is the new Emacs: a Developer Operating System :-)
Posted Nov 1, 2012 3:52 UTC (Thu) by Aliasundercover (subscriber, #69009)
[Link]
> FYI, just checked and Eclipse (at least Juno/4.2) allows this. I can drag a source tab and place it next to my main source view
Hmm, I last poked at Eclipse a while back and got stuck in tile land till I gave up and moved on to try other IDEs which also left me in tiles. I find tiles most unsatisfying as I wind up spending my time fiddling with them and scrolling too small window fragments even on the biggest screen. I much prefer overlapping windows I can customize. I have been working in Kate which lets me arrange things as I like, permits multiple windows into the same source file and shares the screen well with windows from other programs.
Taking another look based on your message I got the Eclipse 3.7 Ubuntu offered me and found I can indeed get decent looking windows if I fiddle enough. It will take time to know if the fiddling stays excessive or I can tame it with more knowledge.
Of course it wants to mangle my code as I type. It really wants me to follow someone's idea of correct style and I know I will be some time getting it to stop helping me by making an awful mess of my code. Why exactly does it insist on extra * characters on each line? (No, don't answer that, I just want to turn it off.)
/*
* I can really do without these fool *s.
* Much preference fussing and still I get *s.
*/
Will have to see. Looking better than last time. Qt Creator and KDevelop never got me this far, not past tiles and tabs with them. So much IDE stuff filling the screen, so little space for the source code I actually care about.
Thanks
Thoughts on the ext4 panic
Posted Oct 31, 2012 9:25 UTC (Wed) by cesarb (subscriber, #6266)
[Link]
> What is it with the IDEs not allowing multiple source windows? [...] I don't see a Linux IDE which allows this.
VIM has split/vsplit. Qt Creator also has split modes. And as others have already noted, Eclipse also has something of the sort. IIRC, Emacs can split the screen too.
Thoughts on the ext4 panic
Posted Oct 31, 2012 13:20 UTC (Wed) by nix (subscriber, #2304)
[Link]
Emacs has always been able to split the screen, since long long before X support existed. (It calls the panes 'windows' and the GUI windows 'frames' because Emacs does everything differently.)
Emacs 24 can split the screen, both manually and automatically, in so many ludicrous ways I have not begun to explore the possibilities. (The window-management code was rewritten by Martin Rudalics, and the new code is... very flexible. Almost too flexible to understand.)
Thoughts on the ext4 panic
Posted Nov 1, 2012 9:32 UTC (Thu) by ncm (subscriber, #165)
[Link]
"So many ways"?
1. Wrong
Any others?
Thoughts on the ext4 panic
Posted Nov 1, 2012 19:50 UTC (Thu) by daglwn (subscriber, #65432)
[Link]
> I don't see a Linux IDE which allows this.
Emacs/CEDET.
Thoughts on the ext4 panic
Posted Oct 30, 2012 10:38 UTC (Tue) by nix (subscriber, #2304)
[Link]
Well, yeah. Given how wide modern monitors are, 80 chars is still silly narrow. I can easily fit *three* 120-char-wide panes side by side. (Now maybe I use a smaller font than most, but even a normal xterm font should be able to fit two.)
Thoughts on the ext4 panic
Posted Oct 30, 2012 5:00 UTC (Tue) by wblew (subscriber, #39088)
[Link]
Interestingly enough, I have not used a terminal window, for source code viewing, that is less than 150 characters for a number of years.
I truly enjoy programming my 1920x1200 27" monitor.
Thoughts on the ext4 panic
Posted Oct 30, 2012 5:01 UTC (Tue) by MTecknology (subscriber, #57596)
[Link]
I love 80 char limits. Thanks to larger screens we can make that a soft limit too! I find that it still makes a crazy about of sense, especially when it comes to readability. I believe that's covered in chapter two of the Linux kernel coding style? "The limit on the length of lines is 80 columns and this is a strongly preferred limit."
Thanks nix! I love that summary. It saves me from having to read anything or do any of my own research and still sound smart when I explain it to others! :D
Thoughts on the ext4 panic
Posted Oct 30, 2012 10:39 UTC (Tue) by nix (subscriber, #2304)
[Link]
I agree with the limit on the length of lines: it should generally be shorter, more like 70 chars. That has nothing to do with terminal emulator width and everything to do with readability and the human eye.
But that width is the length of the line from its first character to its last: you do not sweep your eye over the indentation, so it should not count.
Thoughts on the ext4 panic
Posted Oct 30, 2012 18:53 UTC (Tue) by vonbrand (subscriber, #4458)
[Link]
According to Poynton's "Ten Common Mistakes in the Typesetting of Technical Documents", a 66-character line of text is widely considered ideal for readability. I've seen other claims in the same vein (e.g. in the LaTeX memoir and KOMA packages documentation, aimed at serious book-writing). The 80 characters come from the IBM punched cards of yore, but their design in turn surely wasn't completely random either.
The decree from the $POWERS_THAT_BE is enshrined in the Linux coding style; trying to change that is futile (or at least, there are more fruitful outlets for your creative energies).
Thoughts on the ext4 panic
Posted Oct 30, 2012 21:00 UTC (Tue) by dlang (✭ supporter ✭, #313)
[Link]
80 characters was based on 10 characters per inch on standard typrwriters combined with 8 1/2" wide paper and the fact that you needed margins of ~1/4" to avoid problems with trying to type up to the edge of the paper.
When teletypes were built, they used the same print mechanisms and so had the same limits.
When terminals were built, they mimicked the printed stuff (so that you could see everything that you could see on the paper, and it was a waste to have anything wider, since the people who were still using paper wouldn't be able to see it)
IBM punch cards were 80 columns to match the paper as well.
The problem is none of these are good reasons any longer.
As for the ideal column width to read, go do some research on why newspapers use such narrow columns, the ideal width for reading is surprisingly narrow, and NOT 66 characters.
a paperback book is about the outer edge of what a good width is (no matter what the font size)
Thoughts on the ext4 panic
Posted Nov 4, 2012 23:37 UTC (Sun) by marcH (subscriber, #57642)
[Link]
> As for the ideal column width to read, go do some research on why newspapers use such narrow columns, the ideal width for reading is surprisingly narrow, and NOT 66 characters.
Source code and newspaper articles are quite different types of "literature". The are typically laid out in extremely different ways. It would be a very surprising coincidence if their "ideal widths" were the same.
Thoughts on the ext4 panic
Posted Nov 6, 2012 18:25 UTC (Tue) by anselm (subscriber, #2796)
[Link]
Newspaper columns are as narrow as they are because that lets the publisher fit more stuff on a page and cut paper costs, not because narrow columns are especially easy to read.
Newspapers even go to the trouble of having special fonts designed for them (where do you think Times Roman got its name?) in order to be able to cram more type into a narrow column, thus making the effective column width greater.
Thoughts on the ext4 panic
Posted Oct 30, 2012 9:54 UTC (Tue) by niner (subscriber, #26151)
[Link]
The solution to the horrible formatting of do_one_pass() is not to increase the character limit, but to factor out parts of the function to decrease indentation levels. Some parts are indented 8 levels.
To quote CodingStyle: "The answer to that is that if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."
do_one_pass() is 390 lines long making it very hard to see it's structure anyway which just strengthens the indication of needed refactoring.
Thoughts on the ext4 panic
Posted Oct 30, 2012 10:43 UTC (Tue) by nix (subscriber, #2304)
[Link]
So... you think that if you split each case statement into its own function, it would somehow become better designed?
Sorry, I'm as much a fan of functional decomposition as the next man -- actually I go a bit nuts about it -- but the key to that is splitting functions at logical boundaries. Splitting at random because the indentation passes some arbitrary 'too deep' level, when you would not have split it otherwise, is ridiculous -- and this function indicates that the kernel developers do not in fact do this ridiculous thing. (Obviously there *is* an effective length/complexity limit, and perhaps do_one_pass() is past it -- but a hypothetical function with a long stretch of low indentation with a bit of high indentation in the middle of it is *not* made any clearer by splitting the high indentation into a completely arbitrary separate function!)
Thoughts on the ext4 panic
Posted Oct 30, 2012 11:00 UTC (Tue) by niner (subscriber, #26151)
[Link]
It may become better designed or maybe not. I did not read the code in detail because of too little time to decifer the code.
Maybe I was a bit unclear on this: if there is indeed not a single logical boundary in the function, then well, tough luck. Then this is one of the rare cases where there's just no good solution. Even with an increased character limit, the function would not be that much more readable. It would still be too long to get a good grasp of the structure quickly.
I did not mean that arbitrarily cutting up the function would be a solution, just that usually it is very much possible to decompose and achieve increased readability. That's why I used the word "indication" in my last sentence.
Thoughts on the ext4 panic
Posted Oct 30, 2012 14:14 UTC (Tue) by viro (subscriber, #7872)
[Link]
oh, give me a break...
tagp = pointer; // fairly fat expression, actually
while (tagp - pointer <= size) { // again, and so's 'size'
tag = (journal_block_tag_t *) tagp;
flags = be32_to_cpu(tag->t_flags);
/* couple of lines */
err = jread(&obh, journal, io_block);
if (err) {
success = err;
printk(....);
} else {
/* about 50 lines */
}
skip_write:
tagp += sizeof(journal_block_tag_t);
if (!(flags & JFS_FLAG_SAME_UUID))
tagp += 16;
if (flags & JFS_FLAG_LAST_TAG)
break;
}
In a case. Wrapped in a while. Only at Dibbler's, and that's cutting me own throat...
Seriously, in this case 80-column heuristic has worked nicely - the code structure is badly obfuscated and 4-character tabs would only hide a visible indicator of trouble.
Eight character tabs
Posted Oct 30, 2012 13:01 UTC (Tue) by man_ls (subscriber, #15091)
[Link]
The 80-char wrapping is obsolete, but the 8-char tab rule is just... puzzling. I usually have 4-char tabs and it works fine. I have used 2-char tabs and they work fine most of the time -- and that was for Python where indentation can get a bit hairy without {} blocks to guide the eye. The kernel coding style implies that 8-char tabs are easier to the eye, but that is a bit subjective -- for me they are much harder to read.
Is there any real reason why you cannot display tabs as 4 characters locally in your Linux code? After all Linux does use the tab character. I realize that you risk being burnt alive at the stake in some Linux conference if you forget to change tab length and they spot you, but that is a small price to pay for being able to read code comfortably.
Eight character tabs
Posted Oct 30, 2012 14:32 UTC (Tue) by nix (subscriber, #2304)
[Link]
Changing the local tab width is safe as long as you never ever use spaces to indent anywhere, even on the ends of tabs to line up arguments in multi-line argument lists. That's... rare, and if people proceed to change tab widths and then line up arguments using *that* width, the result is just a mess for everyone. (Just another reason why it is basically a mistake to use tabs for anything. Spaces everywhere! I note that even GNU software, long a weird 8-char-tab-for-indentation-but-four-char-indent holdout, is slowly switching to spaces, project by project, because dealing with tabs has failure cases that just don't exist if you use spaces everywhere.)
Eight character tabs
Posted Oct 30, 2012 14:41 UTC (Tue) by paulj (subscriber, #341)
[Link]
Well, dealing with spaces has failure cases that just don't exist if you use tabs everywhere :). But yes, over all, too many people will get tabs wrong that standardising on spaces - inferior as they are - is the only option.
Eight character tabs
Posted Oct 30, 2012 17:26 UTC (Tue) by nix (subscriber, #2304)
[Link]
I don't see anything wrong with spaces. They make your uncompressed source code 10% or so larger, but these days that has no effect at all on anyone unless you're dealing with a source tree as large as, say, Chromium's. Everyone knows how big a space is, nobody can decide to make it take up more horizontal space... spaces are nice and uncontroversial and always work. Tabs don't.
Eight character tabs
Posted Oct 30, 2012 21:49 UTC (Tue) by man_ls (subscriber, #15091)
[Link]
The problem with spaces is exactly that you cannot change indentation on the fly just by changing the local configuration. Every developer can choose the tabstop that suits them better. It is even possible to have different configurations for different situations: one for the laptop screen, another when the laptop is hooked to a monitor, etcetera. Spaces don't allow that.
Also, it takes longer to press space four times (or eight) than tab once. You can usually configure your favorite editor to insert the spaces when tab is pressed, even to delete them (:set expandtab in vim), so this issue is not so important.
As to the failure case you mention above, it makes sense to indent always using tabs, consistently. I have now remembered my painful days of aligning arguments using spaces -- it is just not worth it. I prefer to use short argument lists and set line wrap at 120 so I don't ever see multi-line argument lists; if they arise then just indent into a new line.
Eight character tabs
Posted Oct 31, 2012 0:42 UTC (Wed) by nix (subscriber, #2304)
[Link]
Unfortunately changing indentation on the fly by changing tabs never, ever works. It only works if all developers are utterly rigorous about only ever indenting with tabs, always indenting with strictly one more tab, never using tabs to line anything up -- and, oh yes, never trying to make anything line up at all. If you like a coding style which lines up wrapped parameters one column after the open bracket of the call on the previous line (a very common style) you cannot do it, even with tabs, and expect indentation-changing-via-tab to work.
I have never worked on a codebase where changing indentation via tab width changes did anything but turn the codebase into goo. It is hopeless.
The way to reindent is via automatic reindentation programs (such as GNU indent). Teach that your indentation style, and you're home free. Nothing else works.
Eight character tabs
Posted Oct 31, 2012 1:05 UTC (Wed) by bronson (subscriber, #4806)
[Link]
... and then somehow unreindent before you commit your code? Otherwise, unless you enforce company-wide indent settings (good luck!), most of your changes will be whitespace churn.
I'm at the point where I'm ready to declare that, on a real world codebase with multiple teammembers, nothing at all works. Nobody likes the whitespace gestapo.
Eight character tabs
Posted Oct 31, 2012 8:52 UTC (Wed) by man_ls (subscriber, #15091)
[Link]
My point of view is a bit different: tabs are good, therefore avoid everything that doesn't work with tabs. Lining things up is evil, hard wrap-up limits are evil, and so on. Yes, it requires a lot of discipline, but then so does software development in general.
Eight character tabs
Posted Oct 31, 2012 13:17 UTC (Wed) by nix (subscriber, #2304)
[Link]
Yeah, automated reindentation before commit only works if you have something pre-commit that reindents again. I've worked on projects with such rules, and it does work (certainly better than what we had before, with people with all sorts of tab widths and odious 'optimizing' editors that translated spaces to tabs of whatever width the user had set throughout the entire file on every save). Better yet is to adapt to whatever the coding standard of the project is and just eat your whining most of the time. (Says the guy who whined about part of the kernel's coding standards just a few posts up. Hypocrisy is *good* for you!)
Eight character tabs
Posted Oct 31, 2012 11:40 UTC (Wed) by etienne (subscriber, #25256)
[Link]
> spaces are nice and uncontroversial and always work. Tabs don't.
Well, have you ever used an editor with variable width font?
Why - maybe because text is easier to read...
Then, you can only use tabs to align stuff - unless that is the beginning of the line. If you comment a line using "//" at its beginning stuff stay aligned.
I do not want a hard limit on the number of chars, because not all chars are the same width...
Also, in some case (long debug printf() line), breaking the long line makes the structure of the function more complex than it really is; just tell you editor not to wrap and just ignore the end of the printf() line - it is just a printf()!
Eight character tabs
Posted Oct 31, 2012 13:21 UTC (Wed) by nix (subscriber, #2304)
[Link]
Variable-width fonts don't work at all for most programming languages for the reasons you state. I actually tried to use a variable-width font only for comments for a while, but it really messes up ASCII-art, and the places where you find ASCII-art in comments are generally the places where the code is complex enough to *need* it, and you don't want to have difficulty piled on by interpreting proportional-font damage too.
Eight character tabs
Posted Oct 31, 2012 17:08 UTC (Wed) by tialaramex (subscriber, #21167)
[Link]
printf() - a function famous for never having been implicated in any complicated security bugs. Oh wait.
No, thanks all the same but I'd rather actually see the code I'm maintaining and not trust that the bits I can't see aren't important.
I like the ASCII control characters, but most of them don't belong in my source code and that includes U+0009 TAB. Use soft tabs, set the continuous integration software to barf on hard tabs in source code, along with mysterious trailing whitespace and similar sins.
Eight character tabs
Posted Oct 31, 2012 18:52 UTC (Wed) by nix (subscriber, #2304)
[Link]
I don't understand your debug printf() comment at all. You can always break a printf() line, even in the format string, with string literal concatenation.
Eight character tabs
Posted Nov 1, 2012 9:41 UTC (Thu) by ncm (subscriber, #165)
[Link]
Me, I've never understood why people insist that printf format strings should be indented at all. I push them to the left margin, and it brings no confusion, but much clarity.
Eight character tabs
Posted Nov 1, 2012 12:06 UTC (Thu) by etienne (subscriber, #25256)
[Link]
But most of the time you want to understand the structure of the function you are about to modify, even if you have (probably commented) printf's showing that you entered the function with those parameters, that you are calling that other function with those parameters, what is the value of each of the fields of that structure...
Having 3/4 lines of code with no algorithm meaning does not help to see the structure of the code and is difficult to comment/uncomment at once; it is easier to tell your editor not to wrap lines and so not display the end of those (probably commented) lines.
Obviously you can write the functions print_mystruct_and_cpt(), print_mystruct_and_idx(), ... that you call only once in a commented out block.
Linux source do not have much "logging" lines, so it is not a real problem there.
Eight character tabs
Posted Oct 30, 2012 18:11 UTC (Tue) by marcH (subscriber, #57642)
[Link]
> But yes, over all, too many people will get tabs wrong that standardising on spaces - inferior as they are - is the only option.
Tab indentation was invented to illustrate "The perfect is the enemy of the good".
Let everyone have his own indentation taste from the same source? Great idea on paper. Falls apart on a regular basis. Because it's not compatible with lining up across lines, because it's not compatible with a max width, etc. And because people are only human and will not be disciplined enough.
Eight character tabs
Posted Oct 31, 2012 11:37 UTC (Wed) by Jonno (subscriber, #49613)
[Link]
Lining up arguments in tab-indented code works just fine, iff you use the same number of tabs for indentation, and then line up the arguments with spaces.
Eight character tabs
Posted Oct 31, 2012 18:09 UTC (Wed) by marcH (subscriber, #57642)
[Link]
"In theory, theory and practice are the same. In practice, they are not."
Thoughts on the ext4 panic
Posted Oct 30, 2012 5:33 UTC (Tue) by shentino (subscriber, #76459)
[Link]
It's possible to make something break without being broken yourself.
Plenty of bugs are caused by one patch exposing a defect in another patch.