On Wed, Jan 15, 2025 at 12:18:13PM -0500, Sasha Levin wrote: > On Wed, Jan 15, 2025 at 12:15:46PM +0100, Simona Vetter wrote: > > On Wed, Jan 15, 2025 at 10:38:34AM +0100, Greg KH wrote: > > So my understanding is that you got confused by this: > > > > > commit 55039832f98c7e05f1cf9e0d8c12b2490abd0f16 upstream > > > > $ git log --grep="(cherry picked from commit 55039832f98c7e05f1cf9e0d8c12b2490abd0f16)" --since="6 month ago" --pretty=oneline > > f0ed39830e6064d62f9c5393505677a26569bb56 > > > > And yes f0ed39830e6064d62f9c5393505677a26569bb56 is the commit you care > > about for stable backport and cve tracking purposes, because it's the one > > in v6.13-rc6. > > > > And the thing is, Sasha's bot found that one too: > > > > https://lore.kernel.org/all/20250110164811-61a12d6905bb8676@xxxxxxxxxxxxxxxxx/ > > > > Except Sasha's bot plays guessing games, the above git log query is exact. > > Cool, can we test it out? I'll try and pick a recent commit (2024). Thanks a lot for specific examples, makes it much easier to walk through them and show how much cherry pick tracing is neeeded. > Let's assume that I'm looking at the v6.10 git tree before 50aec9665e0b > ("drm/xe: Use ordered WQ for G2H handler") made it upstream ("git > checkout v6.10" will do the trick), and I get a backport request that > says: > > commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de upstream > > I run my trusty script that says "50aec9665e0b isn't real, grep for > cherry picked from line!". My trusty script runs the query you've > provided: > > $ git log --grep="(cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de)" --since="6 month ago" --pretty=oneline > 2d9c72f676e6f79a021b74c6c1c88235e7d5b722 drm/xe: Use ordered WQ for G2H handler > c002bfe644a29ba600c571f2abba13a155a12dcd drm/xe: Use ordered WQ for G2H handler > > Which commit do I pick? Note that they are slightly different from > eachother, and c002bfe644 landed in v6.9 while 2d9c72f676 landed in > v6.10. Ok let me first explain why this happens. drm subtrees feature-freeze around -rc6/7, to make sure that when the merge window is open we don't have a buggy chaotic mess but are ready. Which means for that short amount of time there's 3 trees: - drm-intel-next, which aims at v6.11 at this point and contains 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de - drm-intel-next-fixes, which aims for v6.10-rc and contains the cherry-picked version 2d9c72f676e6f79a021b74c6c1c88235e7d5b722 of the same commit - drm-intel-fixes, which aims at v6.9-rc and contains yet another cherry-picked version c002bfe644a29ba600c571f2abba13a155a12dcd of the same commit Now we generally rotate maintainership among releases, so Thomas Hellström was taking care of anything needed for v6.10 and Lucas De Marchi for v6.9, and they both individually did the necessary cherry pick at pretty much the same time. And so we end up with two cherry-picks of the same commit. At other times you might end up with a chain of cherry-picks, it's all the same. Now looking back this is it's very silly, but it's a lot less silly as stuff gets merged into Linus' tree. First v6.9 gets tagged, which contains c002bfe644a29ba600c571f. You don't have that in v6.8.y yet, so you cherry-pick it over. Nothing special here. If you feel like you also assign a CVE for that upstream commit. Then v6.10-rc1 is released, which contains 2d9c72f676e6f79a021b74c6. You don't have that, or a cherry pick of that commit in any of your stable trees, so might be tempted to try to backport it and then realize you seem to have a duplicate of this commit already. But you're not yet done cherry-pick tracing, because 2d9c72f676e6f79a021b7 contains the following line in its commit message: (cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de) So you also need to check whether you have any cherry-picks of 50aec9665e0babd: - v6.9.y contains c002bfe644a29ba600c, so doesn't need another backport of that. You skip this commit. - v6.8.y and older stable trees contain a backport of c002bfe644a29ba6, and if you didn't delete any cherry-picked lines all those backports still contain (cherry picked from commit 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de) line, so you know you're good and don't need another copy of that commit. Since this commit gets completely filtered you're also not tempted to assign a new CVE for this one. So no risk of duplicates. Next up v6.11-rc1 is released, which contains 50aec9665e0babd62b9eee4e613d9a1ef8d2b7de. You check all your stable release branches and notice they all contain a cherry pick already: - v6.10.y has 2d9c72f676e6f79a021b7 - v6.9.y has c002bfe644a29ba600c571f - v6.8.y and older have a cherry pick of c002bfe644a29ba600c571f Again this commit is completely filtered out with cherry-pick tracin checks. Note that except for the first patch none of this mess should ever get past scripted filtering. Which means it should not confuse stable maintainers, and it also should not result in multiple CVEs getting assigned. Because only the first commit (c002bfe644a29ba600c in v6.9) will ever get past all the cherry pick tracing checks. > > Like I tried to explain in my reply to Sasha somewhere else in this thread > > it really only takes two things: > > - drm maintainers consistently add cherry picked from lines anytime we > > cherry-pick > > - you adjust your script to go hunt for the cherry pick alias if you get a > > sha1 that makes no sense, so that you can put in the right sha1. And if > > you do that for any sha1 you find (whether upstream references, Fixes: > > or Reverts or stable candidate commits or whatever really), it will sort > > out all the things we've been shouting about for years now. > > We still have holes here... For example, this backport claims to: > > Fixes: 8135f1c09dd2 ("drm/xe/oa: Don't reset OAC_CONTEXT_ENABLE on OA stream close") > > But 8135f1c09dd2 is a cherry-pick: > > (cherry picked from commit 0c8650b09a365f4a31fca1d1d1e9d99c56071128) > > In the future, if we get a new patch that says: > > Fixes: 0c8650b09a36 ("drm/xe/oa: Don't reset OAC_CONTEXT_ENABLE on OA stream close") > > By your logic, our scripts will look at it and say "0c8650b09a36 is a > real commit, but it's not in linux-6.12.y so there's no need to backport > the fix". > > Which is the wrong thing to do, because we have 8135f1c09dd2 in > linux-6.12.y. > > So no, this isn't a simple trace-the-cherry-pick-tags exercise. So this is not quite what you do, because before you drop this patch you have to check whether you have a cherry-pick of 8135f1c09dd2 in your stable branch. Now if we assume that only the stable team does cherry picks, you can limit your search to just the v6.12.y branch from the v6.12, and you'd wrongly conclude that you don't have a cherry pick there. But since the drm maintainers also do cherry-picks then limiting yourselv to just the patches you've applied to v6.12 will miss some, so you need to cherry-pick trace some more (I think 6 months into the past is generally enough, from your starting point commit). And then you'll find that 8135f1c09dd2 is in v6.12 and a cherry-pick of 0c8650b09a36 so you need that bugfix. > > Automatically, without human intervention, because it's just a git > > oneliner. > > So look at the backport in question which started this thread. The > backporter ends up with: > > """ > [...] > > commit 55039832f98c7e05f1cf9e0d8c12b2490abd0f16 upstream Cherry pick tracing says this is f0ed39830e6064d62f9c5393505677a26569bb56 which is in v6.13-rc1. > > [...] > > Fixes: 8135f1c09dd2 ("drm/xe/oa: Don't reset OAC_CONTEXT_ENABLE on OA stream close") This one is in v6.12, but it's already a cherry-pick, so you need to be careful and look for possible other versions, because its commit message contains (cherry picked from commit 0c8650b09a365f4a31fca1d1d1e9d99c56071128) But further cherry-pick tracing didn't show up any more commits that we didn't know of, so it's all done. And 0c8650b09a365f4a31fca1d1d1e9d99c56071128 itself is in v6.13-rc1, so that resolves. > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> > Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx> # commit 1 > Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # 6.12+ > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > Link: https://patchwork.freedesktop.org/patch/msgid/20241220171919.571528-2-umesh.nerlige.ramappa@xxxxxxxxx > (cherry picked from commit 55039832f98c7e05f1cf9e0d8c12b2490abd0f16) This commit will show up in v6.14-rc1. Currently all you can use this for is as a lookup key to find other cherry-pick copies. > Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> > (cherry picked from commit f0ed39830e6064d62f9c5393505677a26569bb56)o This commit is in v6.13-rc6, because that -rc series also needed the cherry-pick since the broken commit was in v6.13-rc1. And this commit itself is correctly annotated as a cherry-pick of 55039832f98c7e05f1, so it all checks out. > """ > > Where most of the git IDs in it are invalid right now :) They all make sense, but sometimes you do have to do a bit more cherry-pick tracing than what you've done. Sometimes you need to do multiple levels of tracing, sometimes you start at a cherry-pick and need to fish out the original sha1 first (even if that's not one that resolves for you, you can still use it to find more cherry-picks). And because the stable team isn't the only maintainer team doing cherry-picks, you need to broaden your query a bit and can't limit yourself to your own cherry-picks only, to make sure you get them all. Happy to walk you through even more special-case ladden examples. I tried to think them all through when cooking up the committer model years ago, I might indeed have missed a case. But pretty sure that the answer will be "you need more cherry-pick tracing". Cheers, Sima -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
