On Thu, Jun 27, 2024 at 06:32:37PM -0500, Lucas De Marchi wrote:
> On Tue, Jun 25, 2024 at 10:12:05PM GMT, Greg Kroah-Hartman wrote:
> > On Tue, Jun 25, 2024 at 03:02:02PM -0400, Rodrigo Vivi wrote:
> > > > > >> >
> > > > > >> > > > (cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de)
> > > > > >> >
> > > > > >> > Would help out here, but it doesn't.
> > > > > >>
> > > > > >> I wonder if there would be a way of automate this on stable scripts
> > > > > >> to avoid attempting a cherry pick that is already there.
> > > > > >
> > > > > > Please tell me how to do so.
> > > > > >
> > > > > >> But I do understand that any change like this would cause a 'latency'
> > > > > >> on the scripts and slow down everything.
> > > > > >
> > > > > > Depends, I already put a huge latency on drm stable patches because of
> > > > > > this mess. And I see few, if any, actual backports for when I report
> > > > > > FAILED stable patches, so what is going to get slower than it currently
> > > > > > is?
> > >
> > > My thought was on the stable scripts doing something like that.
> > >
> > > For each candidate commit, check if it has the tag line
> > > (cherry picked from commit <original-hash>)
> >
> > Right there, that fails with how the drm tree works. So you are going
> > to have to come up with something else for how to check this. Or fix
> > your process to make this work.
> >
> > Look at the commits tagged for stable in the -rc1 merge window. They
> > don't have the "cherry picked" wording as they were not cherry picked!
> > They were the "originals" that were cherry picked from.
> >
> > > if so, then something like:
> > > if git rev-parse --quiet --verify <original-hash> || \
> > > git log --grep="cherry picked from commit <original-hash> -E --oneline >/dev/null; then
> > > echo "One version of this patch is already in tree. Skipping..."
> > > # send-email?!
> > > else
> > > #attempt to apply the candidate commit...
> > >
> > > > > > Normally you all tag these cherry-picks as such. You didn't do that
> > > > > > here or either place, so there was no way for anyone to know. Please
> > > > > > fix that.
> > >
> > > I'm afraid this is not accurate. Our tooling is taking care of that for us.
> >
> > Then your tooling needs to be fixed.
> >
> > > > > To be fair, this one seems to have been an accident. The same commit was
> > > > > cherry-picked to *two* different branches by two different people
> > > > > [1][2], and this is something we try not to do. Any cherry-picks should
> > > > > go to one tree only, it's checked by our scripts, but it's not race free
> > > > > when two people are doing this roughly at the same time.
> > >
> > > Also I don't believe there's anything wrong here. It was a coincidence on
> > > the timing, but one is
> > > drm-xe-next-fixes-2024-05-09-1
> > > and the other
> > > drm-xe-fixes-2024-05-09
> > >
> > > both maintained by different people at that time.
> > >
> > > >
> > > > any cherry-pick SHOULD have the git id referenced when they are
> > > > cherry-picked, that's what the id is there for. Please always do that.
> > >
> > > Original commit hash is 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de
> > > 50aec9665e0b ("drm/xe: Use ordered WQ for G2H handler")
> >
> > And that's not in Linus's tree.
> >
> > > drm-xe-next-fixes-2024-05-09-1
> > > has commit 2d9c72f676e6 ("drm/xe: Use ordered WQ for G2H handler")
> > > which contains:
> > > (cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de)
> > > Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> > >
> > > drm-xe-fixes-2024-05-09
> > > has commit c002bfe644a2 ("drm/xe: Use ordered WQ for G2H handler")
> > > which contains:
> > > (cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de)
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> >
> > Ok, but again, the original is not in Linus's tree.
>
> current workflow is:
>
> All patches go to drm-xe-next which may be targeting the next kernel
> release or the next->next. Then the fixes (and less frequently, other
> commits which are not important here) are cherry-picked by maintainers
> to the current or next kernel release. In other words, the cherry-pick
> always targets a version 1 less than the original patch.
That's fine, as long as that cherry-pick references the git id of the
original one. Which did not happen here, and does NOT happen at all for
all drm branches (like AMD right now).
> couple of issues I see:
>
> 1) original commit not being in Linus's tree
>
> If you are traversing the branch and you have a "Fixes: XXXX" where XXXX
> is from a previous release, you will most likely see the "cherry picked
> from <original>" *before* the <original> commit showing up in that tree.
>
> Examples looking at the most recent fixes in Linus's tree with
>
> $ git rev-parse origin/master
> 6d6444ba82053c716fb5ac83346202659023044e
>
> $ git log --format="%h %s%n%(trailers)" -n3 --grep "^Fixes:" \
> 6d6444ba82053c716fb5ac83346202659023044e \
> -- drivers/gpu/drm/xe
>
> d21d44dbdde8 in v6.10-rc5, fixes aef4eb7c7dec from v6.9
> but f0ccd2d805e55e12b430d5d6b9acd9f891af455e is not in any tag.
>
> 2470b141bfae in v6.10-rc4, fixes 975e4a3795d4 from v6.8
> but 6800e63cf97bae62bca56d8e691544540d945f53 is not in any tag.
>
> cd554e1e118a in v6.10-rc4, fixes 0698ff57bf32 from v6.10-rc3
> but b321cb83a375bcc18cd0a4b62bdeaf6905cca769 is not in any tag.
>
> Without a change in the workflow, I don't think you can rely on the
> original commit hash being present in Linus's tree, unless you wait one
> entire kernel release cycle.
Which you obviously do not want me to do as you wanted that fix in the
tree sooner.
So this is a pain, but I'm kind of used to it by now as you all seem to
like this workflow for whatever reason...
> 2) Duplicate commits with "cherry picked from ... "
>
> This only happened because it was cherry-picked in 2 branches, both for
> current and for next kernel releases. This can only happen during the
> last rcN as drm-xe-next-fixes is only used there.
>
> I think (2) is easier to fix: we should really add more checks in dim
> to avoid that: if a commit is a candidate to drm-xe-fixes it should
> never be a candidate to drm-xe-next-fixes.
Please fix.
> As for (1), I don't see how it could be fixed without a workflow change.
> That would require committers to add the commits to the right branch
> rather than relying on maintainers to cherry-pick them to the -fixes
> branch after they are merged in drm-xe-next. What dim could do is to
> automate that for the committer and figure out the branch by itself
> based on the 2 commit hashes.
>
> For all the above I used drm-xe-* as example, but it applies to
> drm-intel-* too.
>
> Without the fix to (2) and workflow change from (1), this situation can be
> avoided from the -stable scripts by checking if the commit or any
> cherry-pick thereof is already in the stable tree:
>
> git merge-base --is-ancestor $original_commit HEAD
> git log -n1 --grep "^(cherry picked from commit $original_commit)"
>
> the second search is very expensive though.... probably need to find a
> way to limit the search space.
That's still a major pain, as is your current workflow. So until you
all fix this up, I will continue to complain as the existing way drm
patches flow into stable trees is a major pain which causes me to dread
looking at them every time.
greg k-h
