Re: [PATCH v5] checkpatch: check for missing Fixes tags

[Dan Carpenter]

On Wed, Jun 12, 2024 at 08:46:24AM +0200, Thorsten Leemhuis wrote:
> On 11.06.24 20:38, Andrew Morton wrote:
> > On Tue, 11 Jun 2024 16:43:29 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> > 
> >> This check looks for common words that probably indicate a patch
> >> is a fix.  For now the regex is:
> >>
> >> 	(?:(?:BUG: K.|UB)SAN: |Call Trace:|stable\@|syzkaller)/)
> >>
> >> Why are stable patches encouraged to have a fixes tag?  Some people mark
> >> their stable patches as "# 5.10" etc.  This is useful but a Fixes tag is
> >> still a good idea.
> > 
> > I'd say that "# 5.10" is lame
> 
> Documentation/process/stable-kernel-rules.rst documents this use to
> "Point out kernel version prerequisites".
> 

No, the 5.10 means that the fix is required for everything after 5.10.
Here is how you reference pre-requisites.

    Cc: <stable@xxxxxxxxxxxxxxx> # 3.3.x: a1f84a3: sched: Check for idle
    Cc: <stable@xxxxxxxxxxxxxxx> # 3.3.x: 1b9508f: sched: Rate-limit newidle
    Cc: <stable@xxxxxxxxxxxxxxx> # 3.3.x: fd21073: sched: Fix affinity logic

The documentation was written before we went to 12 character hashes and
also these days we normally put ("") around the subject.  I've made a
copy of all the uses of this format from 2023 at the bottom of this
email to see how people use it in real life.

> > and it would be good if checkpatch could
> > detect this and warn "hey, use a proper Fixes:".  Because
> > 
> >> It  helps people to not cherry-pick buggy patches without also
> >> cherry-picking the fix.
> > 
> > seems pretty important.
> 
> Hmmm. That would lead to false positive when it comes to changes that
> for example just add a device ID (and thus do not "Fix" anything) while
> having prerequisites that are only available in a specific version.

What I'm saying is, imagine you are maintaining a distro kernel for
10 years.  In this scenario you're pulling in whole new wifi drivers
so the kernel still runs on modern hardware.  The stable tag says
"apply this to 6.8+" because that's when the driver was merged.  But as
a distro maintainer it's much nicer to have a Fixes: 123412341234 ("Add
new wifi driver").

regards,
dan carpenter

Dependencies listed in 2023:

    Cc:  <stable@xxxxxxxxxxxxxxx> # 6.1.x: 3837a03 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
    Cc:  <stable@xxxxxxxxxxxxxxx> # 6.1.x: 3837a03 serial: sc16is7xx: improve regmap debugfs by using one regmap per port
    Cc: <stable@xxxxxxxxxxxxxxx> # 6.6+: f8ff234: kernel/Kconfig.kexec: drop select of KEXEC for CRASH_DUMP
    Cc: <stable@xxxxxxxxxxxxxxx> # v6.0+: 1da5c9b x86: Introduce ia32_enabled()
    Cc: stable@xxxxxxxxxxxxxxx # 6.6.x: c5dbf0416000: platform/x86: hp-bioscfg: Simplify return check in hp_add_other_attributes()
    Cc: stable@xxxxxxxxxxxxxxx # 6.6.x: 5736aa9537c9: platform/x86: hp-bioscfg: move mutex_lock() down in hp_add_other_attributes()
    Cc: <stable@xxxxxxxxxxxxxxx> # selftests/resctrl: Refactor feature check to use resource and feature name
    Cc: <stable@xxxxxxxxxxxxxxx> # selftests/resctrl: Remove duplicate feature check from CMT test
    Cc: <stable@xxxxxxxxxxxxxxx> # selftests/resctrl: Move _GNU_SOURCE define into Makefile
    Cc: stable@xxxxxxxxxxxxxxx # 5.9.x: 09252177d5f9: SUNRPC: Handle major timeout in xprt_adjust_timeout()
    Cc: stable@xxxxxxxxxxxxxxx # 5.9.x: 7de62bc09fe6: SUNRPC dont update timeout value on connection reset
    Cc: stable@xxxxxxxxxxxxxxx # 0b035401c570: rbd: move rbd_dev_refresh() definition
    Cc: stable@xxxxxxxxxxxxxxx # 510a7330c82a: rbd: decouple header read-in from updating rbd_dev->header
    Cc: stable@xxxxxxxxxxxxxxx # c10311776f0a: rbd: decouple parent info read-in from updating rbd_dev
    Cc: <stable@xxxxxxxxxxxxxxx> # 6.1.y: bf0207e172703 ("drm/amdgpu: add S/G display parameter")
    Cc: <stable@xxxxxxxxxxxxxxx> # 6.1.y: bf0207e172703 ("drm/amdgpu: add S/G display parameter")
    Cc: <stable@xxxxxxxxxxxxxxx> # 5.15.x: 60a0aab7463ee69 arm64: module-plts: inline linux/moduleloader.h
    Cc: stable@xxxxxxxxxxxxxxx # 588159009d5b: rbd: retrieve and check lock owner twice before blocklisting
    Cc: stable@xxxxxxxxxxxxxxx # f38cb9d9c204: rbd: make get_lock_owner_info() return a single locker or NULL
    Cc: stable@xxxxxxxxxxxxxxx # 8ff2c64c9765: rbd: harden get_lock_owner_info() a bit
    Cc: 6.4+ <stable@xxxxxxxxxxxxxxx> # 6.4+: 8bcbb18c61d6: thermal: core: constify params in thermal_zone_device_register
    Cc: stable@xxxxxxxxxxxxxxx # please backport to all LTSes but not before v6.6-rc2 is tagged
    Cc: stable@xxxxxxxxxxxxxxx      # 3.18: a872ab303d5d: "usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup"
    Cc: stable@xxxxxxxxxxxxxxx # v6.0+ 2f38e84 net/ncsi: make one oem_gma function for all mfr id
    Cc: <stable@xxxxxxxxxxxxxxx> # 6.0: 5365cea199c7 ("soc: qcom: llcc: Rename reg_offset structs to reflect LLCC version")
    Cc: <stable@xxxxxxxxxxxxxxx> # 6.0: c13d7d261e36 ("soc: qcom: llcc: Pass LLCC version based register offsets to EDAC driver")
    Cc: stable@xxxxxxxxxxxxxxx # 6.1.y: 5591a051b86b: drm/amdgpu: refine get gpu clock counter method
    Cc: stable@xxxxxxxxxxxxxxx # 6.2.y: 5591a051b86b: drm/amdgpu: refine get gpu clock counter method
    Cc: stable@xxxxxxxxxxxxxxx # 6.3.y: 5591a051b86b: drm/amdgpu: refine get gpu clock counter method
    Cc: stable@xxxxxxxxxxxxxxx #3.2: 30332eeefec8: debugfs: regset32: Add Runtime PM support
    Cc: <stable@xxxxxxxxxxxxxxx> # dependency for "drm/rockchip: vop: Leave
    Cc: stable@xxxxxxxxxxxxxxx # 4.15: 30332eeefec8: debugfs: regset32: Add Runtime PM support
    Cc: <stable@xxxxxxxxxxxxxxx> # v5.19+ (if someone else does the backport)
    CC: stable@xxxxxxxxxxxxxxx # 5.4.x: c8a5f8ca9a9c: btrfs: print checksum type and implementation at mount time
    Cc: <stable@xxxxxxxxxx> # d6fd48eff750 ("virt/coco/sev-guest: Check SEV_SNP attribute at probe time")
    Cc: <stable@xxxxxxxxxx> # 970ab823743f (" virt/coco/sev-guest: Simplify extended guest request handling")
    Cc: <stable@xxxxxxxxxx> # c5a338274bdb ("virt/coco/sev-guest: Remove the disable_vmpck label in handle_guest_request()")
    Cc: <stable@xxxxxxxxxx> # 0fdb6cc7c89c ("virt/coco/sev-guest: Carve out the request issuing logic into a helper")
    Cc: <stable@xxxxxxxxxx> # d25bae7dc7b0 ("virt/coco/sev-guest: Do some code style cleanups")
    Cc: <stable@xxxxxxxxxx> # fa4ae42cc60a ("virt/coco/sev-guest: Convert the sw_exit_info_2 checking to a switch-case")
    Cc: stable@xxxxxxxxxxxxxxx      # 5.1: 680f8666baf6: interconnect: Make icc_provider_del() return void
    Cc: <stable@xxxxxxxxxx> # 2355370cd941 ("x86/microcode/amd: Remove load_microcode_amd()'s bsp parameter")
    Cc: <stable@xxxxxxxxxx> # a5ad92134bd1 ("x86/microcode/AMD: Add a @cpu parameter to the reloading functions")