Zack's Kernel News

Spanking Linus

Dave Airlie posted an update to the DRM code awhile back. One of the Linux kernel's little jokes was to take the established acronym for Digital Rights Management, which is essentially an anti-open source concept, and repurpose it to mean Direct Rendering Manager, which is the kernel subsystem that deals with video card GPUs.

It was a standard update, except Linus Torvalds got his knuckles rapped at the end of it.

Linus received the patch, saying, "thanks, looks good to me," and life proceeded. The tracker bot announced that the patch had been merged. However, once Linus built the kernel with clang – an alternative to the GNU C Compiler (GCC) – he found that clang reported some fishy looking code in Dave's patch.

It's not really Dave's patch – a bunch of people work on that code, including Dave, and their contributions all get piped up to Linus. He pointed to the problem area, saying "this odd code was introduced by commit 0749ddeb7d6c," and he added, "can we please agree to not write this kind of obfuscated C code?"

Dave replied that he put the relevant person on the job, and that they'd get a fix to Linus. But meanwhile – actually a couple of weeks later – Kirill A. Shutemov reported that Dave's original patch produced a kernel that refused to boot on his system. He identified the specific code that seemed to cause the problem and suggested a workaround; Lyude Paul said he'd get a fix out right away. However, in Lyude's sig file was the following remark, "Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've asked me a question, are waiting for a review/merge on a patch, etc. and I haven't responded in a while, please feel free to send me another email to check on my status."

A few days later, Kirill asked for an update.

Lyude replied that looking at Kirill's patches was "on my TODO list for this week at least as a priority, although I really would have hoped that someone from Intel would have looked by now since it's a regression on their end."

This got the attention of Ville Syrjälä at Intel, who asked what was up. Lyude replied, "whoops, you can ignore this actually – I got this mixed up with an Intel issue I was looking at, this is actually a nouveau issue and you guys don't need to look at this."

So you can start to see what's happening here. Folks are working on stuff, but they are also working on a lot of other stuff at the same time.

Linus, however, replied:

"Christ. It's been two weeks. I'm doing -rc4 today, and I still don't have the fix.

"The problem seems entirely obvious, as reported by Kirill: the nv50 code unconditionally calls the 'atomic_{dis,en}able()' functions, even when not everybody was converted.

"The fix seems to be to either just do the conversion of the remaining cases (which looks like just adding an argument to the remaining functions, and using that for the 'atomic' callback), or the trivial suggestion by Kirill from two weeks ago."

He went on:

"Kirill, since the nouveau people aren't fixing this, can you just send me your tested patch?

"Lyude/Ben – let me just say that I think this is all a huge disgrace.

"You had a problem report with a bisected commit, a suggested fix, and two weeks later there's absolutely _nothing_."

Dave replied, on behalf of Lyude and the rest of the folks who'd contributed to this patch:

"I would like to say when you sent this, there was patches on the mailing lists with Kirill cc'ed, a pull request outstanding to me on the mailing list from Ben, with the patches reviewed in it.

"Maybe you weren't cc'ed on it, but stuff has certainly happened, in the timeframe, and I was keeping track of it from falling down a hole.

"_nothing_ is a lot more a reflection on your research than the ongoing process, there was some delays here and maybe we need to communicate when we are flat out dealing with other more urgent tasks that pay the actual wages."

And that was the end of the discussion.

It's easy for not just Linus, but everyone else, to forget that of the many thousands of people contributing code to the Linux kernel, many of them earn their living doing other things.

There are definitely plenty of people whose sole job is Linux kernel development. They work at companies that are dependent on Linux, and a big chunk of their job is to make sure the kernel supports what they need it to support. But even in those cases, there's a clear difference between those developers' projects and whatever Linus might want them to work on.

The reason it's easy to forget this stuff is because Linux is developed so fast, and with such a broad scope, that it tends to boggle the mind. Linux runs on toasters, vending machines, desktops, server farms, orbiting satellites, and is on track to take over the back end of whatever technology powers the vast simulation that implements the very universe itself. That last item I think is already in beta.

But in fact it really does all boil down to a bunch of passionate people who care a lot, but who don't always have time to do everything they want to do.

Controlling Boot Parameters via Sysfs

Matteo Croce posted a patch to improve user control over reboots. The Linux kernel has a command line like other binaries, with command-line arguments and the whole nine yards, and you can typically set those arguments in the bootloader. Of particular interest to Matteo was controlling whether the system would do a hard boot with full power shutdown and associated tests or a soft boot, which can cut some corners to boot up faster.

Instead of command-line arguments, though, Matteo wanted to expose some sysfs control files that would let the user control it from the previous user session.

Andrew Morton asked why in the holy name of the flying spaghetti monster was this necessary. He didn't ask it like that. He asked it nicer, saying, "Please include a description of why you believe the kernel needs this feature. Use cases, end-user benefits, etc. We've survived this long without it – what changed?"

Matteo replied simply that "we don't always know in advance what type of reboot to perform." He explained, "sometimes a warm reboot is preferred to persist certain memory regions across the reboot. Others a cold one is needed to apply a future system update that makes a memory memory model change, like changing the base page size or resizing a persistent memory region. Or simply we want to enable reboot_force because we noticed that something bad happened."

Meanwhile, Petr Mladek offered some technical suggestions for Matteo's patch, which Matteo promised to implement. He posted a quick update, and Petr gave his "Reviewed-By" tag, signifying that (almost) all seemed well. The two of them did another round of suggestions and updates, and lo and behold Andrew accepted the patch into his tree, in line for submission up to Linus Torvalds and the life eternal.

This question – "why?" – often leads to lively debate. Sometimes the goal of a patch is to implement pseudo-security features that would lock users out of controlling their own systems and give control to some kind of corporate "signing authority" instead. When a patch like that does come along, the debate generally gets kicked off with a developer asking "why?" And after a few back-and-forths, it comes out that yes, indeed, the point of the patch is to give control to the Techmare Bitplane Beast from Beyond. And thus, the patch is not only rejected, but the reasons for its rejection are codified into the historical record.

In this particular instance, Andrew just wanted to know the reasons; having the reasons, he accepted the patch.

Finessing GCC

In the course of a discussion that started off the Linux Kernel Mailing List, some really wonky GCC stuff emerged.

Generally, as one way to catch bugs, the kernel will still try to do something even in code paths that are supposed to be unreachable. Someone implements their favorite feature, which includes all the possible things that might happen, but there's still that final impossible condition that will never happen. So the person writes fall-through code at that point, just in case somehow the kernel decided the impossible was in fact possible. Then if that code ever executes, it'll provide a clue for the developers to fix whatever bug was exposed.

It's a weird yet standard debugging technique. So Peter Zijlstra was talking with Josh Poimboeuf about this exact thing. Apparently the kernel's unreachable() call was being hit. Peter groaned and explained that he had put a workaround in the code, but apparently the workaround had been removed. So Josh remarked, "Oh yeah, I forgot about that. That would be another option if my patch doesn't work out."

At this point, Linus Torvalds came out with the startling interjection, "the problem here is that the compiler fundamentally isn't smart enough to see that something is unreachable, and the 'unreachable()' annotation we did didn't actually really cause any code that makes it so. So we basically have code that _if_ we ever change it, it will simply be wrong, and we'll never see any warnings about it but it will fall through to nonsensical code."

The solution, Linus said, was to write C code that technically would do the exact same thing as the code that was already there, but that actually would force the GCC to generate better binary output and thus avoid ever allowing execution to fall through to the "unreachable" code.

And that was that.

In this particular instance, the solution was to write code that was probably going to look a little simpler than what was there before. The real insanity is when the reverse is true – when the compiler will only produce really great binary output if the source code is absolutely obfuscated and bizarre. I think there may still be parts of the boot-up code that warn developers to steer clear for that reason.