Zack's Kernel News

Adding git Documentation

Documentation is lovely. One especially lovely form of documentation identifies the reasons for using a particular tool in a certain way. Jonathan Corbet recently documented how maintainers should use git to do merges and rebases within the context of feeding patches from many developers up to higher-level maintainers and ultimately up to Linus Torvalds for inclusion in the official kernel tree.

It's no wonder that the situation is tricky. When Linus wrote the git tool, he created a situation in which one group of developers could "pull" an entire tree of development and treat their version essentially as the top of a whole other project. Then other groups of developers could pull that tree into their own sub-projects, and so on, like a spider plant budding little spider plants off of it. This hadn't really existed before, and it's a kind of weird topology, when you consider that earlier revision control systems had none of that. Developers simply contributed to one central repository, with no branching or merging of which to speak – Just one spider plant, but no others budding off it.

As owner of their own sub-projects, maintainers coordinate work among developers coding on that sub-project. To keep things clean, they might do things like edit individual commits after they've already been accepted into the sub-tree or reorder a bunch of commits to have a more logical-seeming history. This is all normal, especially in a large and complex project where the history of the work is crucial for identifying copyright ownership and finding the particular patches that introduced bugs. This editing and reordering is called "rebasing."

Likewise, sub-project maintainers may feed their git tree to an upstream maintainer, hoping that maintainer will "merge" all the patches from that tree into their own, and that they'll then submit the upstream tree to be merged yet further upstream and eventually into Linus's tree, where the patches will become part of the official Linus kernel.

However, merges can go in both directions. The maintainer of a sub-project may want to merge everything that's been going on in an upstream tree into their sub-project's tree, so they'll stay up-to-date with all the work going on in the wider world.

Jonathan's documentation talks about exactly when it's good and bad to merge trees either upstream or downstream, and when it's good to do a rebase – or at least when it should definitely be avoided.

One rule of thumb he identified is very simple, yet also perhaps annoying: Don't rebase any git repository that has already been made public. In other words, only rebase your own private work. Otherwise, you set up all sorts of potential conflicts between trees that have already been merged, but then one tree is rebased and gets merged with the other again; you end up with a big mess instead of the lovely clean history you desired.

In fact, Jonathan said, you really should never rebase at all, if you can help it. Rebasing changes the context in which each patch entered the codebase and could actually introduce bugs. So you should weigh your desire to rebase against your willingness to test all your rebased patches again, even if you had already tested them thoroughly before rebasing.

Merging is a different thing entirely. There's no way to avoid merging – it's one of the main points of using git in the first place. Fork and merge; fork and merge. That's the development process in a nutshell, not just for Linux, but for any project that uses git and has more than a single developer.

As Jonathan said, there were thousands of merge requests just during the Linux 5.1 development cycle. Almost nine percent of all kernel commits were merges.

He also said merges were good for another reason. Linux development history would be clearer if related patches were grouped together in a merge, as opposed to feeding each individual patch into the tree separately. He admonished developers not to bother trying to rebase their trees to avoid merges. A merge was the good case, he said.

This wasn't only true for the top-level tree maintained by Linus. Downstream trees should also accept merges from developers of yet-further-downstream trees, Jonathan said. They should simply add a commit message describing the purpose of the merge and the patches that went into it, just as if it were the commit message for a single patch.

Likewise, he said, for bug hunting, the "Signed-off-by" tags generally used in kernel development should be used in trees going all the way downstream. This would help keep track of who did what and help resolve claims of copyright violation or other legal matters that might arise. As he put it, "Failure to do so threatens the security of the development process as a whole."

Jonathan also cautioned against merging from an upstream tree into a downstream tree that already had unmerged work in it. This could be tempting, if for example you've been working on a new driver for awhile, and your tree has become out of date with all the patches pouring into Linus's official tree; you want to merge all of those changes into your tree, to make sure none of the work you're doing will conflict with anything anyone else has been doing.

It's a perfectly legitimate desire, but Jonathan recommended against it. It would essentially bring a huge X factor of all those many unknown patches into your nice isolated little development branch, which could have a big impact on the behavior of your development kernel; you might have no way of knowing which changes were introduced by problems in your own code versus problems in someone else's.

You might think it would be okay to do a merge from an upstream tree right at the tail end of your development cycle, just to check for conflicts with anyone else's work and resolve those conflicts before submitting your own merge request.

But Jonathan said that wouldn't be good either. As he put it, Linus wanted to see those conflicts himself, so he could go over those parts of the code more carefully before accepting the patches. Conflicts, Jonathan said, identified problem areas that should receive more attention, and not just from the people submitting the patches.

However, Jonathan said it was perfectly fine for you to do a "test merge" (i.e., copy your own branch, merge the upstream tree into that branch, and then check for conflicts yourself). But instead of trying to resolve those conflicts, you should simply discuss their existence when sending your merge request to Linus. That way, he'll know that you know there are issues, and he won't think they somehow slipped by you while he works on resolving them himself.

He closed the doc by saying that, really, these were general guidelines and not laws. Sometimes violating them would be the right thing to do.

There was general approval, especially from Linus, and a few people had some constructive criticism to offer. But the doc is sailing into the kernel tree. It's excellent, because this kind of documentation represents opinion rather than cold hard fact. It documents the preferences and policies of the project leaders and the reasons behind them. Other projects could have different preferences and different reasons.

Untangling the System Call Situation

It's sort of a pain in the neck to change the kernel's system calls. Each supported architecture needs to be updated, and then the lookup tables need to be updated – so the number of each system call can be paired with the actual kernel function being called – and then of course all the other system calls may need to be renumbered. Since there are thousands of people all pouring patches into the kernel at lightning speed, it's likely someone else has also renumbered the system calls, which will cause conflicts between your two patches.

David Howells had the perfectly reasonable impulse to write a script that would handle all the nightmare elements of this process for you. It would update the tables and renumber the other system calls. It would even resolve patch conflicts with other developers. It had all the fixins.

Arnd Bergmann gave his mixed reaction very clearly, when he said that the script looked great – except for the fact that it was needed at all.

Linus Torvalds went the other way, saying the script would have been great – except that what was really needed was something completely different. He said:

"I really think the solution is not this kind of helper script, but simply that we should work at not having each architecture add new system calls individually in the first place.

IOW, we should look at having just one unified table for new system call numbers, and aim for the per-architecture ones to be for 'legacy numbering'.

Maybe that won't happen, but in the hope that it happens, I really would prefer that people not work at making scripts for the current nasty situation."

And that was that. Linus generally makes no bones about the fact that he'll happily inconvenience developers, even while going to tremendous lengths not to inconvenience users. Binary interfaces that might be in use by a single piece of existing compiled code, for example, are sacrosanct. Interfaces that developers use all the time, however, might just go out the window if there's a decent reason. And in the current case, interfaces that are a vicious trial to update will remain a vicious trial, until someone comes up with a better underlying design.

Bit or Bitmap?

One way to get clarification on a feature requirement is to do it wrong and wait for someone to squawk. Thierry Reding may or may not have done that very thing recently, when he posted a patch to update the core driver code so it wouldn't give a warning that he felt might confuse users.

The warning came about when the kernel failed to probe a given driver before reaching the end of its init process during boot time. The probe() function was supposed to initialize all devices and get information about the resources it offered to the system. Without that initialization and information, the system wouldn't be able to use the device. Ordinarily, it would be appropriate for the kernel to give a warning in that circumstance.

However, in some cases, a device couldn't be initialized, because it was a hot-plug device or for various other legitimate reasons. And in that case, a driver that depended on that device might need to defer its probe() call until the device came online – which could be at any point after bootup.

In that case, Thierry said, it would be better to let the driver tell the kernel that was deferring its probe() call until further notice that no warning would be needed – this would be business-as-usual as far as the user was concerned.

Thierry's patch added a flag to some kernel functions, to allow the calling routine to specify that probe() was being deferred. This flag was in the form of a bitmap (i.e., a string of 1s and  s). And this, it turned out, was the cue for a squawk.

Rob Herring and Rafael J. Wysocki had no real objection to the patch, and all seemed well.

Greg Kroah-Hartman, on the other hand, had already spoken to Thierry about this particular feature and had given what he felt had been clear instructions about how the patch should be implemented. Instructions that, in his opinion, Thierry had simply ignored.

Greg had told Thierry to avoid "odd flags," after Thierry's first version of the patch had used a Boolean instead of a bitmap. The bitmap, Greg now said, "did not make the API any easier to understand at all." And he added, "do it correctly please, like I asked for the first time."

Thierry pointed out that Greg had really only specified "no Boolean flags" and had not really been clear about what would be better. He suggested, "to avoid further back and forth, what exactly is it that you would have me do? That is, what do you consider to be the correct way to do this?"

As an alternative to his flagging solution, he proposed some new code that relied on return values and clearly named function calls to indicate that probe() was being deferred. And after some modifications suggested by Rafael, Greg was mollified. Mostly he wanted the interface to be readable and clear and not to require anyone to "just know" the meaning of some binary (or bitmapped) flag. He said, "Yes, that's much more sane. Self-describing APIs are the key here; I did not want a Boolean flag, or any other flag, as part of the public API as they do not describe what the call does at all."

The culture of kernel development is full of personalities, with many different approaches to how to get things done. Sometimes knowing how to get the full attention of the upstream maintainer can be the quickest way to get your patch into the kernel.