Skip to contentSkip to conversation

need to back port ("scsi: ufs: Unlock on a couple error paths")

DISCUSSION Dan Carpenter 3 participants 5 messages May 4, 2021 – May 6, 2021 Stable branch

lore
Share & tools
lore
#0 lore raw

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 <[email protected]>
Date:   Mon Dec 9 10:13:08 2019 -0800
    scsi: ufs: Avoid busy-waiting by eliminating tag conflicts

    [ Upstream commit 7252a3603015f1fd04363956f4b72a537c9f9c42 ]
                      ^^^^^^^^^^^^

regards,
dan carpenter

#1 lore raw

Hi Dan!

Quoted 2 lines

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.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
#2 lore raw

handling Fixes tags on rebased trees

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")

#3 lore raw

Re: handling Fixes tags on rebased trees

Quoted 6 lines · Dan Carpenter

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.

Quoted 7 lines

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

#4 lore raw
In reply to Dan Carpenter
Quoted 19 lines · Dan Carpenter

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 <[email protected]>
Date:   Mon Dec 9 10:13:08 2019 -0800
Quoted 6 lines
    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