Hi,
I ran Smatch on 5.4.116 and I found that we were missing commit bb14dd1564c9 ("scsi: ufs: Unlock on a couple error paths").
The problem was caused because somehow my Fixes tag did not match the upstream commit that stable used. I have both hashes in my git tree and the patches are identical except for the hash. I don't know git well enough to say what went wrong. I don't think the SCSI tree rebases?
My fixes tag: Fixes: a276c19e3e98 ("scsi: ufs: Avoid busy-waiting by eliminating tag conflicts") ^^^^^^^^^^^^
Stable hash: commit a8d2d45c70c7391386baf7863674f156da56a3d5 Author: Bart Van Assche bvanassche@acm.org Date: Mon Dec 9 10:13:08 2019 -0800
scsi: ufs: Avoid busy-waiting by eliminating tag conflicts
[ Upstream commit 7252a3603015f1fd04363956f4b72a537c9f9c42 ] ^^^^^^^^^^^^ regards, dan carpenter
Hi Dan!
I don't know git well enough to say what went wrong. I don't think the SCSI tree rebases?
I try very hard to avoid it but it happens. Bart's patch went into 5.6 and I still have the old SHA you referenced in my repo:
$ git branch --contains a276c19e3e98 5.6/scsi-queue-old
Given the '-old' suffix I assume I had to rebase for some reason.
These days I have a separate, ephemeral staging branch that I use to let patches simmer and the static checkers to do their thing. Once I get a sufficient amount of success emails from the various bots I shuffle things over to the proper scsi-queue branch. At that point the SHA should be stable.
I try to be very careful about updating any commit references when I move things out of staging. My git hook script verifies that any SHAs referenced in patch descriptions are ancestors of both Linus' tree and the branch I'm currently on. If a referenced SHA is not an ancestor of any of those trees, the commit fails. I do something like this:
$ git merge-base --is-ancestor a276c19e3e98 linus/master ; echo $? 1 # Not in Linus' tree
$ git merge-base --is-ancestor 7252a3603015 linus/master ; echo $? 0 # In Linus' tree
A quick git blame on my git hook reveals that I implemented the SHA ancestry validation in 2019 shortly after the Bart's commit was merged. Not sure if I added the check due to that discrepancy. Certainly possible.
It turns that rebasing without updating the Fixes tag is sort of common. I wrote a script to find the invalid tags from the last month and have include the output below. Two of the patches are in -mm and presumably Andrew is going fold the Fixes commit into the original commit when these are sent upstream so those aren't a real issue.
We could probably try catching rebased trees when they are merged in linux-next? I'll play with this and see if it works. But we're going to end up missing some. Maybe we need a file with a mapping of rebased hashes which has something like:
28252e08649f 0df68ce4c26a ("iscv: Prepare ptdump for vm layout dynamic addresses") 42ae341756da d338ae6ff2d8 ("userfaultfd: add minor fault registration mode")
regards, dan carpenter
#!/usr/bin/perl
open HASHES, '-|', 'git log --since="1 month ago" --grep="Fixes:" --pretty=format:"%h"' or die $@;
my $hash; while (defined($hash = <HASHES>)) { chomp($hash); my @commit_msg=`git show --pretty="%b" -s $hash`;
foreach my $line (@commit_msg) { if ($line =~ /^Fixes: ([0-9a-f]*?) /) { my $fix_hash = $1; if (system("git merge-base --is-ancestor $fix_hash linux-next")) { print "$hash $line"; } } } } close HASHES;
Here is the output, of invalid fixes tag in the last month.
28252e08649f Fixes: e9efb21fe352 ("riscv: Prepare ptdump for vm layout dynamic addresses") 42ae341756da Fixes: f2bf15fb0969 ("userfaultfd: add minor fault registration mode") eda5613016da Fixes: 5b109cc1cdcc ("hugetlb/userfaultfd: forbid huge pmd sharing when uffd enabled") 85021fe9d800 Fixes: 1ace37b873c2 ("drm/amdgpu/display: Implement functions to let DC allocate GPU memory") caa93d9bd2d7 Fixes: 855b35ea96c4 ("usb: common: move function's kerneldoc next to its definition") 0f66f043d0dc Fixes: cabcebc31de4 ("cifsd: introduce SMB3 kernel server") 3ada5c1c27ca Fixes: 788b6f45c1d2 ("cifsd: add server-side procedures for SMB3") 0e672f306a28 Fixes: 6788fa154546 ("veth: allow enabling NAPI even without XDP") aec00aa04b11 Fixes: 830027e2cb55 ("KEYS: trusted: Add generic trusted keys framework") ef32e0513a13 Fixes: 67982dfa59de ("usb: cdns3: imx: add power lost support for system resume")
On Thu, May 06, 2021 at 12:39:41PM +0300, Dan Carpenter wrote:
It turns that rebasing without updating the Fixes tag is sort of common. I wrote a script to find the invalid tags from the last month and have include the output below. Two of the patches are in -mm and presumably Andrew is going fold the Fixes commit into the original commit when these are sent upstream so those aren't a real issue.
We could probably try catching rebased trees when they are merged in linux-next? I'll play with this and see if it works. But we're going to end up missing some. Maybe we need a file with a mapping of rebased hashes which has something like:
28252e08649f 0df68ce4c26a ("iscv: Prepare ptdump for vm layout dynamic addresses") 42ae341756da d338ae6ff2d8 ("userfaultfd: add minor fault registration mode")
I thought Stephen's scripts already catch the "this commit isn't in the tree" issue? I use them when I take patches, so that logic came from somewhere :)
thanks,
greg k-h
On Tue, May 04, 2021 at 09:46:35PM +0300, Dan Carpenter wrote:
Hi,
I ran Smatch on 5.4.116 and I found that we were missing commit bb14dd1564c9 ("scsi: ufs: Unlock on a couple error paths").
The problem was caused because somehow my Fixes tag did not match the upstream commit that stable used. I have both hashes in my git tree and the patches are identical except for the hash. I don't know git well enough to say what went wrong. I don't think the SCSI tree rebases?
My fixes tag: Fixes: a276c19e3e98 ("scsi: ufs: Avoid busy-waiting by eliminating tag conflicts") ^^^^^^^^^^^^
Stable hash: commit a8d2d45c70c7391386baf7863674f156da56a3d5 Author: Bart Van Assche bvanassche@acm.org Date: Mon Dec 9 10:13:08 2019 -0800
scsi: ufs: Avoid busy-waiting by eliminating tag conflicts [ Upstream commit 7252a3603015f1fd04363956f4b72a537c9f9c42 ] ^^^^^^^^^^^^regards, dan carpenter
Thanks for catching this, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Dan Carpenter -
Greg KH -
Martin K. Petersen