Lab 8: A Patch Series (Milestone 9)

Background

One patch is a sentence. A series is an argument: an ordered set of changes where each step is individually reviewable, individually revertable, and leaves the tree working.

That last property — every commit builds and boots — is what makes git bisect work for everyone who comes after you, and it is a hard requirement rather than a nicety. A series that breaks the build in the middle is a landmine for whoever bisects a regression through it in 2031.

This lab produces a real series, verifies it bisects, sends it, and takes it through a v2.

Why This Lab Matters

• Any change worth doing is usually more than one patch.
• Splitting well is the skill that most visibly separates a first-time contributor from a regular.
• The v2 cycle — collecting trailers, writing an honest changelog, proving with range-diff — is where trust is built or lost.
• A series is where the ordering rules stop being abstract, because getting them wrong breaks the build and you see it.

Prerequisites

• Lab 7 sent (it need not have landed).
• Patch Craft read, especially the series section.
• b4 installed and working.
• A subsystem remote configured, and a working presend.sh from Lab 7.

Predict First

1. You have a change that adds a helper and uses it in three drivers. How many patches, and in what order?
2. What happens to git bisect if patch 3 of 5 does not compile? Be specific about the mechanism.
3. You reworked the locking in v2. What must happen to the Reviewed-by: you collected on v1?
4. Which is more likely to get review: a 4-patch series or a 25-patch series? Why?
5. How long does git rebase --exec 'make' take on your machine for a 5-patch series? Guess, then measure.

The Target

   [PATCH 0/4] cover letter — the argument for the whole thing
     ├── [PATCH 1/4] refactor: extract the helper       no behavior change
     ├── [PATCH 2/4] add the new capability             enabled by 1/4
     ├── [PATCH 3/4] use it in the driver               the actual point
     └── [PATCH 4/4] selftest: cover the new behavior   proves 3/4

   Every one of 1..4 builds and boots.
   No patch depends on a later one.
   Any prefix of the series is a working tree.

Step-by-Step Tasks

Step 1: Find work that genuinely needs splitting

Good candidates, in rough order of how easy they are to find:

cd ~/kernel/linux

# Duplicated logic — a refactor waiting to happen:
rg -n "the_repeated_pattern" drivers/<area>/ | head -20

# Uses of a deprecated API in one subsystem:
$EDITOR Documentation/process/deprecated.rst
rg -n "strlcpy|kmalloc\(.*\*.*sizeof" drivers/<area>/ | head

# Something Coccinelle finds:
make coccicheck MODE=report M=drivers/<area>/ 2>&1 | head -40

Tip: A conversion series (one patch per driver, converting a deprecated pattern) is an excellent second contribution: the change is mechanical and reviewable, the split is obvious, and maintainers value it. It also forces you to read a lot of drivers, quickly.

Step 2: Split it

Apply the two questions from patch craft to every hunk:

1. Could this half be reverted without the other?
2. Would a reviewer want to say yes to one and no to the other?

Then build the series with git rebase -i, or by committing in the right order from the start.

git switch -c my-series <subsys>/<branch>
# ...commit in order: preparation first, the point last...
git log --oneline <subsys>/<branch>..HEAD

Step 3: Prove it bisects

This is the step that makes it a series rather than a pile of patches.

BASE=<subsys>/<branch>

# Every commit compiles:
git rebase --exec "make -s -j$(nproc) O=../build" "$BASE"

# Every commit compiles AND boots:
git rebase --exec "make -s -j$(nproc) O=../build && ~/kernel-labs/scripts/rig-check.sh" "$BASE"

If the rebase stops, the commit it stopped on is broken. Fix it in place:

# ...fix the source...
git add -u
git commit --amend --no-edit
git rebase --continue

Warning: It is tempting to fix a mid-series build break by squashing the fix into a later patch, or by moving code between patches until it compiles. Do neither reflexively — a series that only compiles because of an arbitrary shuffle usually means the split is wrong. Re-ask the two questions; the right split almost always compiles at every step naturally.

Step 4: Write the cover letter

git format-patch --cover-letter --base=auto --thread=shallow "$BASE"..HEAD
$EDITOR 0000-cover-letter.patch

The template is empty and *** SUBJECT HERE *** is a real thing people forget to replace. Four headings:

Subject: [PATCH 0/4] subsys: support the thing

PROBLEM
What is wrong or missing today, and who it affects. Someone who has never
seen this code should be able to tell whether they care.

APPROACH
Why this shape. Patches 1-2 prepare; patch 3 is the actual change; patch 4
is the test. If you considered another design and rejected it, say so — it
saves the round trip where a reviewer suggests it.

NOT ADDRESSED
What this deliberately does not do, and why. This prevents the review
comment "but what about X" from being a surprise.

TESTING
What you built, what you ran, on what, with which debug options.
"x86_64 and arm64 under QEMU, KASAN and lockdep enabled, selftest passes,
allmodconfig clean" is worth more than three paragraphs of reasoning.

Based on <tree> commit abc1234 ("...").

Step 5: Check the whole series

./scripts/checkpatch.pl --strict "$BASE"..HEAD          # or -g
~/kernel-labs/scripts/presend.sh "$BASE"..HEAD

# The bots will do these; do them first:
make O=../b-all allmodconfig && make O=../b-all -j"$(nproc)"
make O=../b-clang LLVM=1 defconfig && make O=../b-clang LLVM=1 -j"$(nproc)"
make O=../build C=2 W=1 <your/dirs>/

# Sanity on the produced files:
grep -c '^Signed-off-by:' 0*.patch          # one per patch
head -30 0000-cover-letter.patch            # did you actually write it?
grep '^base-commit:' 0*.patch

Step 6: Send it

./scripts/get_maintainer.pl 0*.patch

git send-email --dry-run --to=... --cc=... 0*.patch     # read the output
git send-email --to=... --cc=... 0*.patch

--thread=shallow (the default from format-patch) makes every patch a reply to the cover letter, not a chain. Check on lore that the thread looks like a fan, not a ladder.

Step 7: Handle review and produce v2

# 1. Collect every trailer people gave you, mechanically.
b4 trailers -u

# 2. Make the changes. Rebase onto the current tip.
git fetch <subsys> && git rebase <subsys>/<branch>

# 3. Re-verify bisectability — a rebase can break it.
git rebase --exec "make -s -j$(nproc) O=../build" <subsys>/<branch>

# 4. Prove v2 contains exactly what you claim.
git range-diff <subsys>/<branch>..<v1-tip> <subsys>/<branch>..HEAD

# 5. Regenerate.
git format-patch -v2 --cover-letter --base=auto --thread=shallow <subsys>/<branch>..HEAD

The changelog goes under the --- of the cover letter (and per-patch, if a patch changed):

---
Changes in v2:
 - Use copy_struct_from_user() so older userspace still works (Arnd)
 - Reject unknown flag bits rather than masking them (Arnd)
 - Split the whitespace fix out into a separate patch (Greg)
 - Dropped Jane's Reviewed-by on 3/4: the locking changed substantially

v1: https://lore.kernel.org/all/<v1-cover-message-id>/

Then reply in the v1 thread saying v2 is out, with a link. Reviewers do not watch for new threads.

Implementation Requirements / Deliverables

• A series of 3–8 patches implementing one coherent change.
• Each patch is one logical change, with its own justification in its own message.
• git rebase --exec 'make' passes across the whole series — run, not assumed.
• No patch depends on a later one; any prefix is a working tree.
• A cover letter with all four headings, actually written.
• --base=auto on every patch.
• checkpatch --strict clean across the series.
• allmodconfig and a clang build both completed.
• Sent, correctly threaded under the cover letter (verified on lore).
• A v2 produced after review, with:
  • trailers collected with b4 trailers -u
  • a changelog under --- that credits who asked for what
  • a range-diff you actually read
  • any stale Reviewed-by: dropped, and said so
  • a reply in the v1 thread pointing at v2
• The deliberate-break experiment performed and written up.

Expected Output

$ git format-patch --cover-letter --base=auto --thread=shallow net-next/main..HEAD
0000-cover-letter.patch
0001-subsys-extract-the-helper.patch
0002-subsys-add-the-capability.patch
0003-subsys-use-it-in-the-driver.patch
0004-selftests-subsys-cover-the-new-behavior.patch

$ git rebase --exec "make -s -j8 O=../build" net-next/main
Executing: make -s -j8 O=../build
Executing: make -s -j8 O=../build
Executing: make -s -j8 O=../build
Executing: make -s -j8 O=../build
Successfully rebased and updated refs/heads/my-series.

On lore, the thread is a fan and not a ladder:

[PATCH 0/4] subsys: support the thing
  ├─ [PATCH 1/4] subsys: extract the helper
  ├─ [PATCH 2/4] subsys: add the capability
  ├─ [PATCH 3/4] subsys: use it in the driver
  └─ [PATCH 4/4] selftests: subsys: cover the new behavior

And range-diff, which is what an honest v2 looks like:

$ git range-diff net-next/main..v1 net-next/main..HEAD
1:  abc1234 = 1:  def5678 subsys: extract the helper
2:  bcd2345 ! 2:  efg6789 subsys: add the capability
    @@ -42,7 +42,7 @@
     -	if (copy_from_user(&karg, uarg, sizeof(karg)))
     -		return -EFAULT;
     +	ret = copy_struct_from_user(&karg, sizeof(karg), uarg, usize);
     +	if (ret)
     +		return ret;
3:  cde3456 = 3:  fgh7890 subsys: use it in the driver
4:  def4567 = 4:  ghi8901 selftests: subsys: cover the new behavior

= means unchanged, ! means changed. If a patch shows ! and your changelog does not mention it, one of the two is wrong.

Debugging Steps

git rebase --exec stops in the middle

That commit does not build. Fix it in place with git commit --amend --no-edit, then git rebase --continue. If several commits break, your split is wrong — re-ask the two questions.

The thread on lore is a ladder, not a fan

You sent with chained reply-to. format-patch --thread=shallow (the default) plus git send-email --no-chain-reply-to fixes it.

Patch 1 of 4 arrived but the others did not

Some mail providers rate-limit. git send-email has --batch-size and --relogin-delay:

git send-email --batch-size=5 --relogin-delay=10 --to=... 0*.patch

b4 trailers -u finds nothing

It matches on the message-ID of what you sent. If you regenerated the patches after sending, or sent from a different tree, the link is broken — b4 trailers -u --since=1-month sometimes helps, and otherwise you are back to copying by hand, carefully.

A reviewer says "this should be one patch"

The opposite failure: you split something that has no independent meaning. A patch that cannot be described without referring to the next one belongs with it.

range-diff shows a change you did not intend

A rebase silently absorbed something, or you amended the wrong commit. This is exactly the error range-diff exists to catch — investigate before sending.

The cover letter has *** SUBJECT HERE *** in it

It happens to everyone once. It is also visible forever. Read what you produced.

Experiment

CLAIM. A non-bisectable series is not an aesthetic problem. It actively breaks a real tool for real people, and the damage is permanent.

METHOD.

1. Take your series and deliberately break the middle: move a definition from patch 2 into patch 3, so patch 2 does not compile.
2. Confirm it: git rebase --exec 'make -s -j$(nproc) O=../build' <base> stops.
3. Now add a fifth commit that introduces an obvious runtime bug, and write a one-line test script that detects it.
4. Run a bisection across the whole series:

git bisect start HEAD <base>
git bisect run sh -c 'make -s -j$(nproc) O=../build && ./detect-bug.sh'

PREDICTION. Before running: what does git bisect run do when the build fails at an intermediate commit? Does it (a) report that commit as the culprit, (b) skip it automatically, (c) abort, or (d) give a wrong answer?

RESULT. Record what happened, and what a bisecting stranger would have had to do manually.

Then read what the exit code 125 means and why it exists:

man git-bisect | sed -n '/125/,+12p'

Finally, fix the series and re-run the bisection. Write one sentence about the difference in effort — that sentence is the argument you will make to someone who wants to skip this step.

Test

cat > ~/kernel-labs/scripts/series-check.sh <<'EOF'
#!/usr/bin/env bash
# Everything checkable about a SERIES before it leaves your machine.
set -euo pipefail
KTREE="${KTREE:-$HOME/kernel/linux}"
BASE="${1:?usage: series-check.sh <base-ref>}"
cd "$KTREE"
rc=0

n=$(git rev-list --count "$BASE"..HEAD)
echo "== $n patches on top of $BASE"
[ "$n" -ge 2 ] || { echo "  not a series"; exit 2; }
[ "$n" -le 15 ] || echo "  WARNING: $n patches is a lot; consider two series"

echo "== every commit builds"
git rebase --exec "make -s -j$(nproc) O=../build" "$BASE" || rc=1

echo "== checkpatch"
./scripts/checkpatch.pl --strict -g "$BASE"..HEAD || rc=1

echo "== each commit has a sign-off and a non-empty body"
for c in $(git rev-list "$BASE"..HEAD); do
  b=$(git show -s --format='%b' "$c")
  echo "$b" | grep -q '^Signed-off-by:' || { echo "  no Signed-off-by: $c"; rc=1; }
  # A body that is ONLY trailers means the message explains nothing.
  echo "$b" | grep -qv '^[A-Za-z-]*-by:\|^Fixes:\|^Link:\|^Cc:\|^$' \
    || { echo "  message body is only trailers: $c"; rc=1; }
done

echo "== cover letter and base-commit"
rm -rf /tmp/series && git format-patch --cover-letter --base=auto -o /tmp/series "$BASE"..HEAD >/dev/null
grep -q 'SUBJECT HERE' /tmp/series/0000-* && { echo "  cover letter not written"; rc=1; }
grep -q '^base-commit:' /tmp/series/0001-* || { echo "  no base-commit"; rc=1; }

[ $rc -eq 0 ] && echo "SERIES OK" || echo "SERIES HAS PROBLEMS"
exit $rc
EOF
chmod +x ~/kernel-labs/scripts/series-check.sh
~/kernel-labs/scripts/series-check.sh <subsys>/<branch>

Verify it can fail: break the middle commit and re-run; empty a commit body to only trailers and re-run; leave *** SUBJECT HERE *** in the cover and re-run. All three must be caught.

Challenge Extensions

1. Do the whole thing with b4. b4 prep --new, b4 prep --edit-cover, b4 send, then b4 trailers -u and b4 send again for v2. The cover letter lives in git, which solves the "where did my cover letter go between versions" problem entirely.

2. A conversion series with Coccinelle. Write a semantic patch that finds a deprecated pattern, generate the changes with MODE=patch, and split them per-driver or per-subsystem. Include the .cocci file in the cover letter so reviewers can verify your transformation rather than reading every hunk.

3. Take it to v3. Most real series go to v3 or beyond. Getting there — collecting, rebasing, re-verifying, changelogging — until the mechanics are automatic is the actual deliverable of this lab.

4. Split someone else's patch. Find a patch on lore that was asked to be split, do the split yourself locally, and compare with what the author eventually sent. Free calibration on someone else's round trip.

5. Measure the review latency by series length. For one subsystem, sample twenty series on lore and plot patches-in-series against time-to-first-review. Then decide how long your next series should be.

6. Add a per-patch changelog. For a long-lived series, put the per-patch changes under each patch's own --- as well as in the cover. Reviewers who only care about patch 3 will thank you.

Validation / Self-check

1. Give the two questions that decide whether something is one patch or two, and apply them to your own series out loud.
2. State the bisectability rule and the exact command that proves it.
3. What does git bisect run do at a commit that does not build? What is exit code 125 for?
4. In what order do patches go, and what arrangement is always wrong?
5. What four things must a cover letter answer?
6. What happens to text under ---, and name two things that belong there.
7. Why does a v2 changelog credit reviewers by name?
8. What does git range-diff show, and what specific mistake does it catch?
9. When must you drop a Reviewed-by: in v2, and how do you communicate it?
10. Where does v2 go — a new thread or the old one — and what must you do in the old thread either way?
11. Why is a 25-patch series less likely to get review than a 5-patch one?
12. Your series builds at every commit but a reviewer says "this should be one patch". What went wrong?

Next: Lab 9 — Review a Patch — the one people skip, and the one that changes how you are seen.