7.2: Reviewing patches

Some readers will certainly object to putting this section with "advanced topics" on the grounds that even beginning kernel developers should be reviewing patches. It is certainly true that there is no better way to learn how to program in the kernel environment than by looking at code posted by others. In addition, reviewers are forever in short supply; by looking at code you can make a significant contribution to the process as a whole.

Reviewing code can be an intimidating prospect, especially for a new kernel developer who may well feel nervous about questioning code - in public - which has been posted by those with more experience. Even code written by the most experienced developers can be improved, though. Perhaps the best piece of advice for reviewers (all reviewers) is this: phrase review comments as questions rather than criticisms. Asking "how does the lock get released in this path?" will always work better than stating "the locking here is wrong."

Different developers will review code from different points of view. Some are mostly concerned with coding style and whether code lines have trailing white space. Others will focus primarily on whether the change implemented by the patch as a whole is a good thing for the kernel or not. Yet others will check for problematic locking, excessive stack usage, possible security issues, duplication of code found elsewhere, adequate documentation, adverse effects on performance, user-space ABI changes, etc. All types of review, if they lead to better code going into the kernel, are welcome and worthwhile.