docs: backporting: address feedback

This addresses a few comments/issues in my v2 submission:

- repeated word: 'run' from kernel test robot
- emacs/ediff mode from Jon Corbet
- various comments from Willy Tarreau
- more backporting advice from Ben Hutchings
- a couple more cherry-pick tips from Harshit Mogalapalli
- add a bit about stable submissions

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Link: https://lore.kernel.org/r/20231023135722.949510-1-vegard.nossum@oracle.com
Signed-off-by: Jonathan Corbet <corbet@lwn.net>

1 files changed, 121 insertions, 31 deletions

diff --git a/Documentation/process/backporting.rst b/Documentation/process/backporting.rst
index 7593980af96596..e1a6ea0a1e8a17 100644
--- a/Documentation/process/backporting.rst
+++ b/Documentation/process/backporting.rst
@@ -54,6 +54,12 @@ problem with applying the patch to the "wrong" base is that it may pull
 in more unrelated changes in the context of the diff when cherry-picking
 it to the older branch.
 
+A good reason to prefer ``git cherry-pick`` over ``git am`` is that git
+knows the precise history of an existing commit, so it will know when
+code has moved around and changed the line numbers; this in turn makes
+it less likely to apply the patch to the wrong place (which can result
+in silent mistakes or messy conflicts).
+
 If you are using `b4`_. and you are applying the patch directly from an
 email, you can use ``b4 am`` with the options ``-g``/``--guess-base``
 and ``-3``/``--prep-3way`` to do some of this automatically (see the
@@ -112,6 +118,7 @@ each method, and sometimes there's value in using both.
 We will not cover using dedicated merge tools here beyond providing some
 pointers to various tools that you could use:
 
+-  `Emacs Ediff mode <https://www.emacswiki.org/emacs/EdiffMode>`__
 -  `vimdiff/gvimdiff <https://linux.die.net/man/1/vimdiff>`__
 -  `KDiff3 <http://kdiff3.sourceforge.net/>`__
 -  `TortoiseMerge <https://tortoisesvn.net/TortoiseMerge.html>`__
@@ -255,7 +262,7 @@ something like::
     >>>>>>> <commit>... title
 
 This is what you would see if you opened the file in your editor.
-However, if you were to run run ``git diff`` without any arguments, the
+However, if you were to run ``git diff`` without any arguments, the
 output would look something like this::
 
     $ git diff
@@ -320,11 +327,11 @@ style, which looks like this::
     >>>>>>> <commit> (title)
 
 As you can see, this has 3 parts instead of 2, and includes what git
-expected to find there but didn't. Some people vastly prefer this style
-as it makes it much clearer what the patch actually changed; i.e., it
-allows you to compare the before-and-after versions of the file for the
-commit you are cherry-picking. This allows you to make better decisions
-about how to resolve the conflict.
+expected to find there but didn't. It is *highly recommended* to use
+this conflict style as it makes it much clearer what the patch actually
+changed; i.e., it allows you to compare the before-and-after versions
+of the file for the commit you are cherry-picking. This allows you to
+make better decisions about how to resolve the conflict.
 
 To change conflict marker styles, you can use the following command::
 
@@ -370,6 +377,42 @@ get them out of the way; this also lets you use ``git diff HEAD`` to
 always see what remains to be resolved or ``git diff --cached`` to see
 what your patch looks like so far.
 
+Dealing with file renames
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+One of the most annoying things that can happen while backporting a
+patch is discovering that one of the files being patched has been
+renamed, as that typically means git won't even put in conflict markers,
+but will just throw up its hands and say (paraphrased): "Unmerged path!
+You do the work..."
+
+There are generally a few ways to deal with this. If the patch to the
+renamed file is small, like a one-line change, the easiest thing is to
+just go ahead and apply the change by hand and be done with it. On the
+other hand, if the change is big or complicated, you definitely don't
+want to do it by hand.
+
+As a first pass, you can try something like this, which will lower the
+rename detection threshold to 30% (by default, git uses 50%, meaning
+that two files need to have at least 50% in common for it to consider
+an add-delete pair to be a potential rename)::
+
+  git cherry-pick -strategy=recursive -Xrename-threshold=30
+
+Sometimes the right thing to do will be to also backport the patch that
+did the rename, but that's definitely not the most common case. Instead,
+what you can do is to temporarily rename the file in the branch you're
+backporting to (using ``git mv`` and committing the result), restart the
+attempt to cherry-pick the patch, rename the file back (``git mv`` and
+committing again), and finally squash the result using ``git rebase -i``
+(see the `rebase tutorial`_) so it appears as a single commit when you
+are done.
+
+.. _rebase tutorial: https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec
+
+Gotchas
+-------
+
 Function arguments
 ~~~~~~~~~~~~~~~~~~
 
@@ -385,6 +428,9 @@ Error handling
 If you cherry-pick a patch that includes a ``goto`` statement (typically
 for error handling), it is absolutely imperative to double check that
 the target label is still correct in the branch you are backporting to.
+The same goes for added ``return``, ``break``, and ``continue``
+statements.
+
 Error handling is typically located at the bottom of the function, so it
 may not be part of the conflict even though could have been changed by
 other patches.
@@ -398,31 +444,31 @@ on either of the branches that you're backporting from or to. By
 including the whole function in the diff you get more context and can
 more easily spot problems that might otherwise go unnoticed.
 
-Dealing with file renames
-~~~~~~~~~~~~~~~~~~~~~~~~~
-
-One of the most annoying things that can happen while backporting a
-patch is discovering that one of the files being patched has been
-renamed, as that typically means git won't even put in conflict markers,
-but will just throw up its hands and say (paraphrased): "Unmerged path!
-You do the work..."
-
-There are generally a few ways to deal with this. If the patch to the
-renamed file is small, like a one-line change, the easiest thing is to
-just go ahead and apply the change by hand and be done with it. On the
-other hand, if the change is big or complicated, you definitely don't
-want to do it by hand.
-
-Sometimes the right thing to do will be to also backport the patch that
-did the rename, but that's definitely not the most common case. Instead,
-what you can do is to temporarily rename the file in the branch you're
-backporting to (using ``git mv`` and committing the result), restart the
-attempt to cherry-pick the patch, rename the file back (``git mv`` and
-committing again), and finally squash the result using ``git rebase -i``
-(see the `rebase tutorial`_) so it appears as a single commit when you
-are done.
-
-.. _rebase tutorial: https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec
+Refactored code
+~~~~~~~~~~~~~~~
+
+Something that happens quite often is that code gets refactored by
+"factoring out" a common code sequence or pattern into a helper
+function. When backporting patches to an area where such a refactoring
+has taken place, you effectively need to do the reverse when
+backporting: a patch to a single location may need to be applied to
+multiple locations in the backported version. (One giveaway for this
+scenario is that a function was renamed -- but that's not always the
+case.)
+
+To avoid incomplete backports, it's worth trying to figure out if the
+patch fixes a bug that appears in more than one place. One way to do
+this would be to use ``git grep``. (This is actually a good idea to do
+in general, not just for backports.) If you do find that the same kind
+of fix would apply to other places, it's also worth seeing if those
+places exist upstream -- if they don't, it's likely the patch may need
+to be adjusted. ``git log`` is your friend to figure out what happened
+to these areas as ``git blame`` won't show you code that has been
+removed.
+
+If you do find other instances of the same pattern in the upstream tree
+and you're not sure whether it's also a bug, it may be worth asking the
+patch author. It's not uncommon to find new bugs during backporting!
 
 Verifying the result
 ====================
@@ -503,6 +549,50 @@ as you would (or should) give to any other patch. Having unit tests and
 regression tests or other types of automatic testing can help increase
 the confidence in the correctness of a backport.
 
+Submitting backports to stable
+==============================
+
+As the stable maintainers try to cherry-pick mainline fixes onto their
+stable kernels, they may send out emails asking for backports when when
+encountering conflicts, see e.g.
+<https://lore.kernel.org/stable/2023101528-jawed-shelving-071a@gregkh/>.
+These emails typically include the exact steps you need to cherry-pick
+the patch to the correct tree and submit the patch.
+
+One thing to make sure is that your changelog conforms to the expected
+format::
+
+  <original patch title>
+  
+  [ Upstream commit <mainline rev> ]
+  
+  <rest of the original changelog>
+  [ <summary of the conflicts and their resolutions> ]
+  Signed-off-by: <your name and email>
+
+The "Upstream commit" line is sometimes slightly different depending on
+the stable version. Older version used this format::
+
+  commit <mainline rev> upstream.
+
+It is most common to indicate the kernel version the patch applies to
+in the email subject line (using e.g.
+``git send-email --subject-prefix='PATCH 6.1.y'``), but you can also put
+it in the Signed-off-by:-area or below the ``---`` line.
+
+The stable maintainers expect separate submissions for each active
+stable version, and each submission should also be tested separately.
+
+A few final words of advice
+===========================
+
+1) Approach the backporting process with humility.
+2) Understand the patch you are backporting; this means reading both
+   the changelog and the code.
+3) Be honest about your confidence in the result when submitting the
+   patch.
+4) Ask relevant maintainers for explicit acks.
+
 Examples
 ========