acked-by

Defining the Reviewed-by Tag

Submitted by Jeremy
on October 8, 2007 - 8:07pm
Linux news

"Last month, at the kernel summit, there was discussion of putting a Reviewed-by: tag onto patches to document the oversight they had received on their way into the mainline," began Jonathan Corbet in an effort to define the meaning of the recently introduced reviewed-by tag. He continued, "that tag has made an occasional appearance since then, but there has not yet been a discussion of what it really means. So it has not yet brought a whole lot of value to the process."

In the continued discussion, it was requested that all commit tags be defined, prompting Jonathan to update his documentation to include Signed-off-by, Acked-by, Cc, and Tested-by along with his documentation for Reviewed-by. He offered the following definition for the new Reviewed-by tag:

"The patch has been reviewed and found acceptible according to the Reviewer's Statement as found at the bottom of this file. A Reviewed-by tag is a statement of opinion that the patch is an appropriate modification of the kernel without any remaining serious technical issues. Any interested reviewer (who has done the work) can offer a Reviewed-by tag for a patch."

Introducing Reviewed-by Tags

Submitted by Jeremy
on September 7, 2007 - 12:37pm
Linux news

"Some people seem to be using 'Acked-by' to mean, 'seems good to me', without necessarily doing a full review of the patch, and instead of trying to change the meaning of 'Acked-by', [the plan is] to have a new sign off which is a bit more explicitly about what it means," Theodore Tso explained in a recent thread on the Linux Kernel mailing list. He continued:

"This was proposed by Andrew and discussed at the Kernel Summit; the basic idea is that it is a formal indication that the person has done a *full* review of the patch (a few random comments from the local whitespace police don't count), and is willing to vouch that the patch is correct, safe, extremely unlikely to cause regressions, etc. If the patch does need to be reverted or fixed because it was buggy, then both the original submitter and the reviewer would bear responsibility and subsystem maintainers might take that into account when assessing the reputations of the submitter and reviewer in the future when deciding whether or not to accept a patch."

Andrew Morton noted that the idea isn't fully fleshed out yet, "we will start introducing Reviewed-by: (I haven't yet quite worked out how yet) but it will be a quite formal thing and it would be something which the reviewer explicitly provided. For now, let's please stick with acked-by". Theodore added, "there was also some discussion about whether or not patches would not be accepted at all without a Reviewed-by, but that probably won't happen initially. The general consensus was to gently ease into it and see how well it works first."

Linux: Revisiting Swap Prefetch

Submitted by Jeremy
on July 11, 2007 - 11:35am
Linux news

Another thread discussed potentially merging the swap prefetch patch into the mainline Linux kernel. Con Kolivas [story] started the thread saying "I fixed all bugs I could find and improved it as much as I could last kernel cycle. Put me and the users out of our misery and merge it now or delete it forever please." Replying to an off-list message, Andrew Morton asked users of the patch, "please provide us more details on your usage and testing of that code. Amount of memory, workload, observed results, etc?"

Nick Piggin [interview] noted that he's still interested in better understanding and possibly fixing what's happening with swap and reclaim on the systems reporting a benefit from the swap-prefetch patch. He went on to note, "regarding swap prefetching. I'm not going to argue for or against it anymore because I have really stopped following where it is up to, for now. If the code and the results meet the standard that Andrew wants then I don't particularly mind if he merges it. It would be nice if some of you guys would still report and test problems with reclaim when prefetching is turned off -- I have never encountered the morning after sluggishness (although I don't doubt for a minute that it is a problem for some)." Ingo Molnar followed up to these coments acking the patch, "I have tested it and have read the code, and it looks fine to me. (i've reported my test results elsewhere already [story]) We should include this in v2.6.23."

Linux: Using Acked-by Tags

Submitted by Jeremy
on June 7, 2007 - 10:56am
Linux news

Andrew Morton submitted some documentation explaining the use of the "Signed-off-by" and "Acked-by" tags added when patches are submitted for conclusion into the Linux kernel. "The Signed-off-by: tag implies that the signer was involved in the development of the patch, or that he/she was in the patch's delivery path," the documentation explains, "if a person was not directly involved in the preparation or handling of a patch but wishes to signify and record their approval of it then they can arrange to have an Acked-by: line added to the patch's changelog." When asked about the possibility of including "Tested-by" tags, Andrew replied, "I think it's very useful information to have. For a start, it tells you who has the hardware and knows how to build a kernel. So if you're making a change to a driver and want it tested, you can troll the file's changelog looking for people who might be able to help."

The thread went on to discuss if Ack and Nack patches were useful from non-maintainers. Andrew suggested that a without additional information they don't offer much, "it's better to just provide constructive, detailed technical comments and from that it becomes pretty obvious to all parties whether or not the patch has a future. If you did properly provide that useful feedback then the 'ack' or 'nack' bit becomes redundant." He went on to stress the need for useful feedback, "frankly, I don't trust a simple 'ack' much at all. It's the kernel equivalent of 'whoa, kewl!'"