* [GIT PULL] RAS updates for 5.18 @ 2022-03-23 17:29 Borislav Petkov 2022-03-25 20:01 ` Linus Torvalds 2022-03-25 21:07 ` pr-tracker-bot 0 siblings, 2 replies; 7+ messages in thread From: Borislav Petkov @ 2022-03-23 17:29 UTC (permalink / raw) To: Linus Torvalds; +Cc: x86-ml, lkml Hi Linus, please pull the RAS side of things for 5.18. But before you do, a git question if I may: So the branch I'm sending you is tip/ras/core and I fast-forwarded it to v5.17-rc4 before adding new stuff ontop. However, I needed to have prerequisite work from another tip branch: tip/locking/core which was fast-forwarded to v5.17-rc1 before it got that prerequisite work added ontop. So I merged tip/locking/core into tip/ras/core - see merge commit below - and added the RAS stuff ontop. However, when creating the diffstat for the pull request, it would add additional files to it from tip/locking/core even if all the tip/locking/core changes are already in your master branch: $ git log master..tip/locking/core $ After poking at it a bit more, I found a hint as to what it might be complaining about: $ git diff --stat master...ras_core_for_v5.18_rc1 warning: master...ras_core_for_v5.18_rc1: multiple merge bases, using 754e0b0e35608ed5206d6a67a791563c631cec07 and: $ git merge-base -a master ras_core_for_v5.18_rc1 754e0b0e35608ed5206d6a67a791563c631cec07 - that's rc4 b008893b08dcc8c30d756db05c229a1491bcb992 - that's the top-commit of tip/locking/core at the time the merge commit was created. So it looks like the diffstat for the pull request is created by using the -rc4 as the merge base: $ git diff --stat v5.17-rc4..ras_core_for_v5.18_rc1 ... while the correct diffstat should be from the merge-commit onwards: $ git diff --stat c0f6799de2a0..ras_core_for_v5.18_rc1 ... So, long story short, what am I doing wrong? Thx a lot! --- The following changes since commit 754e0b0e35608ed5206d6a67a791563c631cec07: Linux 5.17-rc4 (2022-02-13 12:13:30 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/ras_core_for_v5.18_rc1 for you to fetch changes up to 7f1b8e0d6360178e3527d4f14e6921c254a86035: x86/mce: Remove the tolerance level control (2022-02-23 11:09:25 +0100) ---------------------------------------------------------------- - More noinstr fixes - Add an erratum workaround for Intel CPUs which, in certain circumstances, end up consuming an unrelated uncorrectable memory error when using fast string copy insns - Remove the MCE tolerance level control as it is not really needed or used anymore ---------------------------------------------------------------- Borislav Petkov (3): Merge tip:locking/core into tip:ras/core x86/mce: Use arch atomic and bit helpers x86/mce: Remove the tolerance level control Jue Wang (1): x86/mce: Work around an erratum on fast string copy instructions Auto-merging MAINTAINERS Auto-merging arch/x86/kernel/cpu/mce/core.c Merge made by the 'ort' strategy. Documentation/ABI/removed/sysfs-mce | 37 ++++++++++++++++ Documentation/ABI/testing/sysfs-mce | 32 -------------- Documentation/vm/hwpoison.rst | 2 - Documentation/x86/x86_64/boot-options.rst | 9 +--- arch/x86/kernel/cpu/mce/core.c | 175 ++++++++++++++++++++++++++++++++++++++++++++------------------------------- arch/x86/kernel/cpu/mce/internal.h | 31 +++++++++++--- arch/x86/kernel/cpu/mce/severity.c | 23 +++++----- 7 files changed, 177 insertions(+), 132 deletions(-) create mode 100644 Documentation/ABI/removed/sysfs-mce -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-23 17:29 [GIT PULL] RAS updates for 5.18 Borislav Petkov @ 2022-03-25 20:01 ` Linus Torvalds 2022-03-25 20:40 ` Borislav Petkov 2022-03-25 21:07 ` pr-tracker-bot 1 sibling, 1 reply; 7+ messages in thread From: Linus Torvalds @ 2022-03-25 20:01 UTC (permalink / raw) To: Borislav Petkov; +Cc: x86-ml, lkml [ I've written this kind of reply multiple times before and I _thought_ we had something in the docs about this, but I can't find them, so here goes again ] On Wed, Mar 23, 2022 at 10:29 AM Borislav Petkov <bp@suse.de> wrote: > > But before you do, a git question if I may: > [.. details removed for brevity..] > However, when creating the diffstat for the pull request, it would > add additional files to it from tip/locking/core even if all the > tip/locking/core changes are already in your master branch: So what you are describing is a very fundamental thing - your branch has multiple separate starting points, since you had different branches that you merged into your tree.. Sometimes having multiple branches doesn't actually cause that, because the different branches may all have the same base starting point. Git calls these things "merge bases", because those starting points is what you have to take into account when merging, they are the "base" for actually resolving the differences that come in through multiple branches. And git handles that perfectly fine when merging by doing all the appropriate magic. And "git log" has no problem with it either - you can list all the commits that are in your head but are *not* in some arbitrary number of merge bases just fine). But when you do a "git diff", things are different (and "git request-pull" basically just does a diff to show what the thing was about). A "diff" is fundamentally something you do on two end-points. You have a beginning, you have an end, and you ask "what changed between these two end-points". But that fundamentally means that when you have multiple different merge bases, and you ask "what changed since the beginning and the current state", your question is fundamentally ambiguous. There is not a "the beginning". There are *multiple* beginnings. So what git will do it to pick _one_ beginning, and just use that. And that means that yes, the diff will show the changes since that beginning, but since the end result depends on the _other_ beginning too, it will show the changes that came from that other beginning as well. Sometimes those changes end up being empty, because the "first beginning" might already have had all of that. So sometimes you might not even notice that what "git diff" gave you was ambiguous. So "git request-pull" does both a log (for the shortlog of commits) and a diff (for the diffstat), and the log should always be correct, but the diffstat will have this ambiguity problem if you have multiple merge bases. > After poking at it a bit more, I found a hint as to what it might be > complaining about: > > $ git diff --stat master...ras_core_for_v5.18_rc1 > warning: master...ras_core_for_v5.18_rc1: multiple merge bases, using 754e0b0e35608ed5206d6a67a791563c631cec07 Yeah, so using that "..." format will warn about how there are multiple possible merge bases, and point out how it just picked one at random. > So it looks like the diffstat for the pull request is created by using > the -rc4 as the merge base: > > $ git diff --stat v5.17-rc4..ras_core_for_v5.18_rc1 > ... > > while the correct diffstat should be from the merge-commit onwards: No. There is no such thing as a *correct* merge base. You had two, and git picked one of them. In the general case there just isn't a correct answer. Now, it turns out that you shouldn't have done a merge at all. I'm not sure why that commit c0f6799de2a0 ("Merge tip:locking/core into tip:ras/core") even exists, because it could have been done as a fast-forward. Did you use "--no-ff" to explicitly not do that? So *because* you have that pointless merge that could just have been a fast-forward, you think that "hey, if it had just picked the other merge base it would all have been fine". But in a normal merge situation, the two merge bases would both have had some work that wasn't in the other side, so that's just because you did something odd. > So, long story short, what am I doing wrong? So in the general case, you aren't doing anything wrong: if you merge multiple real branches, it's just that "git diff" cannot find a single unique point to use as the base, and you'll get some odd random diff. But if you are a developer who merges multiple real branches, you obviously know how to merge things, and one way to sort it out is to basically do a test-merge just for yourself: # WWLS? ("What would Linus See?") git branch -b test-merge linus git merge my-branch git diff -C --stat --summary ORIG_HEAD.. .. save that away .. # go back to your real work, and remove that test-merge git checkout <normal-branch> git branch -D test-merge will generate a diffstat of what a merge (which fundamentally knows how to resolve multiple merge bases) would generate. (Obviously you can just do the above in a completely separate git tree too, if you don't like doing those temporary branches that might mess up your working tree). The other alternative is to just send me the bogus diffstat - I'm sadly quite used to it, since a number of people just do "git request-pull", see that it's odd, don't understand why, and just let me sort it out. Now the good news is that people who are afraid of merges and the above kind of complexity will never actually see this situation. You can't get multiple merge bases if you don't do any merges yourself. So this kind of git complexity only happens to people who are supposed to be able to handle it. You clearly figured out what was going on, you didn't perhaps just realize the full story. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-25 20:01 ` Linus Torvalds @ 2022-03-25 20:40 ` Borislav Petkov 2022-03-25 20:51 ` Linus Torvalds 2022-03-28 14:21 ` Jonathan Corbet 0 siblings, 2 replies; 7+ messages in thread From: Borislav Petkov @ 2022-03-25 20:40 UTC (permalink / raw) To: Linus Torvalds; +Cc: x86-ml, lkml On Fri, Mar 25, 2022 at 01:01:15PM -0700, Linus Torvalds wrote: > [ I've written this kind of reply multiple times before and I > _thought_ we had something in the docs about this, but I can't find > them, so here goes again ] Thanks! I had a faint notion that I had read you telling people that their diffstat was bogus but I couldn't find anything relevant for the short time I was searching. How about I start a maintainers-specific documentation in Documentation/process/ - we already have maintainer handbooks there - and put that there? I'm sure it'll come up again and it'll be easier to point to it next time... > On Wed, Mar 23, 2022 at 10:29 AM Borislav Petkov <bp@suse.de> wrote: <snip detailed explanation - thanks for taking the time!> > But that fundamentally means that when you have multiple different > merge bases, and you ask "what changed since the beginning and the > current state", your question is fundamentally ambiguous. There is not > a "the beginning". There are *multiple* beginnings. Aaaha, there it is. I suspected it was something fundamental... > So what git will do it to pick _one_ beginning, and just use that. > > And that means that yes, the diff will show the changes since that > beginning, but since the end result depends on the _other_ beginning > too, it will show the changes that came from that other beginning as > well. Right. ... > No. There is no such thing as a *correct* merge base. You had two, and > git picked one of them. In the general case there just isn't a correct > answer. > > Now, it turns out that you shouldn't have done a merge at all. I'm not > sure why that commit c0f6799de2a0 ("Merge tip:locking/core into > tip:ras/core") even exists, because it could have been done as a > fast-forward. Did you use "--no-ff" to explicitly not do that? Well, let me recreate the situation: $ git checkout -b ras/core v5.17-rc4 Switched to a new branch 'ras/core' $ git merge tip/locking/core Auto-merging MAINTAINERS Merge made by the 'ort' strategy. MAINTAINERS | 1 + arch/x86/include/asm/cpumask.h | 10 ++++++++++ arch/x86/include/asm/ptrace.h | 2 +- include/asm-generic/bitops/instrumented-atomic.h | 12 ++++++------ include/asm-generic/bitops/instrumented-non-atomic.h | 16 ++++++++-------- include/linux/atomic/atomic-arch-fallback.h | 38 +++++++++++++++++++++++++++++++++----- include/linux/cpumask.h | 18 +++++++++--------- include/linux/jump_label.h | 13 ++++--------- include/linux/local_lock_internal.h | 6 +++--- init/Kconfig | 1 + kernel/locking/lockdep.c | 43 +++++++++++++++++++++++++------------------ kernel/locking/lockdep_internals.h | 6 ++++-- kernel/locking/lockdep_proc.c | 51 +++++++++++++++++++++++++++++++++++++++++++-------- kernel/locking/percpu-rwsem.c | 5 +++-- kernel/locking/rwsem.c | 2 +- scripts/atomic/fallbacks/read_acquire | 11 ++++++++++- scripts/atomic/fallbacks/set_release | 7 ++++++- 17 files changed, 168 insertions(+), 74 deletions(-) so that tip/locking/core branch is based on v5.17-rc1 and has locking, etc stuff which I needed in ras/core. Thus the merge. And git does a merge commit. If I try to make it do a --ff, it still does a merge commit: $ git merge --ff tip/locking/core Auto-merging MAINTAINERS Merge made by the 'ort' strategy. MAINTAINERS | 1 + arch/x86/include/asm/cpumask.h | 10 ++++++++++ arch/x86/include/asm/ptrace.h | 2 +- include/asm-generic/bitops/instrumented-atomic.h | 12 ++++++------ include/asm-generic/bitops/instrumented-non-atomic.h | 16 ++++++++-------- include/linux/atomic/atomic-arch-fallback.h | 38 +++++++++++++++++++++++++++++++++----- include/linux/cpumask.h | 18 +++++++++--------- include/linux/jump_label.h | 13 ++++--------- include/linux/local_lock_internal.h | 6 +++--- init/Kconfig | 1 + kernel/locking/lockdep.c | 43 +++++++++++++++++++++++++------------------ kernel/locking/lockdep_internals.h | 6 ++++-- kernel/locking/lockdep_proc.c | 51 +++++++++++++++++++++++++++++++++++++++++++-------- kernel/locking/percpu-rwsem.c | 5 +++-- kernel/locking/rwsem.c | 2 +- scripts/atomic/fallbacks/read_acquire | 11 ++++++++++- scripts/atomic/fallbacks/set_release | 7 ++++++- 17 files changed, 168 insertions(+), 74 deletions(-) so I don't see how to do a fast-forward thing here. > So *because* you have that pointless merge that could just have been a > fast-forward, you think that "hey, if it had just picked the other > merge base it would all have been fine". But in a normal merge > situation, the two merge bases would both have had some work that > wasn't in the other side, so that's just because you did something > odd. Right. I guess my strategy for the future should be: either make sure branches have a common merge base or generate a "fake" diffstat. > So in the general case, you aren't doing anything wrong: if you merge > multiple real branches, it's just that "git diff" cannot find a single > unique point to use as the base, and you'll get some odd random diff. Right. > But if you are a developer who merges multiple real branches, you > obviously know how to merge things, and one way to sort it out is to > basically do a test-merge just for yourself: > > # WWLS? ("What would Linus See?") > git branch -b test-merge linus > git merge my-branch > git diff -C --stat --summary ORIG_HEAD.. > .. save that away .. > > # go back to your real work, and remove that test-merge > git checkout <normal-branch> > git branch -D test-merge > > will generate a diffstat of what a merge (which fundamentally knows > how to resolve multiple merge bases) would generate. Yes, as a matter of fact I did that before sending you this email and the diffstat it issued when doing the "git merge my-branch" into your tree was the one I was expecting. I guess yeah, that's the way I should be creating the diffstat when I have this situation in the future. Thx! > The other alternative is to just send me the bogus diffstat - I'm > sadly quite used to it, since a number of people just do "git > request-pull", see that it's odd, don't understand why, and just let > me sort it out. Yeah, unlikely. I wanted to know what is going on so you got this email with a question instead. :-) > Now the good news is that people who are afraid of merges and the > above kind of complexity will never actually see this situation. You > can't get multiple merge bases if you don't do any merges yourself. > > So this kind of git complexity only happens to people who are supposed > to be able to handle it. You clearly figured out what was going on, > you didn't perhaps just realize the full story. Thanks for taking the time and explaining - it was very helpful! -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-25 20:40 ` Borislav Petkov @ 2022-03-25 20:51 ` Linus Torvalds 2022-03-28 14:21 ` Jonathan Corbet 1 sibling, 0 replies; 7+ messages in thread From: Linus Torvalds @ 2022-03-25 20:51 UTC (permalink / raw) To: Borislav Petkov; +Cc: x86-ml, lkml On Fri, Mar 25, 2022 at 1:40 PM Borislav Petkov <bp@suse.de> wrote: > > If I try to make it do a --ff, it still does a merge commit: Oh, they indeed aren't fast-forwards of each other, they just looked superficially that way to me because when I did my gitk ORIG_HEAD.. after merging, the fact that I had already merged everything else in both of those branches. So never mind. It wasn't a pointless fast-forward merge, it's just that neither of those branches had anything new in them as far as I was concerned any more. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-25 20:40 ` Borislav Petkov 2022-03-25 20:51 ` Linus Torvalds @ 2022-03-28 14:21 ` Jonathan Corbet 2022-03-28 15:56 ` Borislav Petkov 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Corbet @ 2022-03-28 14:21 UTC (permalink / raw) To: Borislav Petkov, Linus Torvalds; +Cc: x86-ml, lkml Borislav Petkov <bp@suse.de> writes: > On Fri, Mar 25, 2022 at 01:01:15PM -0700, Linus Torvalds wrote: >> [ I've written this kind of reply multiple times before and I >> _thought_ we had something in the docs about this, but I can't find >> them, so here goes again ] > > Thanks! > > I had a faint notion that I had read you telling people that their > diffstat was bogus but I couldn't find anything relevant for the short > time I was searching. > > How about I start a maintainers-specific documentation in > Documentation/process/ - we already have maintainer handbooks there - > and put that there? Maybe something that looks like: https://lore.kernel.org/lkml/87wnghd78t.fsf@meer.lwn.net/ :) Thanks, jon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-28 14:21 ` Jonathan Corbet @ 2022-03-28 15:56 ` Borislav Petkov 0 siblings, 0 replies; 7+ messages in thread From: Borislav Petkov @ 2022-03-28 15:56 UTC (permalink / raw) To: Jonathan Corbet; +Cc: Linus Torvalds, x86-ml, lkml On Mon, Mar 28, 2022 at 08:21:51AM -0600, Jonathan Corbet wrote: > Maybe something that looks like: > > https://lore.kernel.org/lkml/87wnghd78t.fsf@meer.lwn.net/ > > :) I was just about to scratch something together but it is a good thing I got distracted by other stuff so that you'll document it a lot more eloquently than me! :-) A typo: "So what is to be done? The best response when confronted with this situation is to indeed to a merge," should be "... to indeed *do* a merge, ..." Other than that, very much Acked-by: Borislav Petkov <bp@suse.de> Thanks for doing that! -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] RAS updates for 5.18 2022-03-23 17:29 [GIT PULL] RAS updates for 5.18 Borislav Petkov 2022-03-25 20:01 ` Linus Torvalds @ 2022-03-25 21:07 ` pr-tracker-bot 1 sibling, 0 replies; 7+ messages in thread From: pr-tracker-bot @ 2022-03-25 21:07 UTC (permalink / raw) To: Borislav Petkov; +Cc: Linus Torvalds, x86-ml, lkml The pull request you sent on Wed, 23 Mar 2022 18:29:38 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git tags/ras_core_for_v5.18_rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/636f64db07f33a18630248b4c57e182cd315b0da Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-03-28 15:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-23 17:29 [GIT PULL] RAS updates for 5.18 Borislav Petkov 2022-03-25 20:01 ` Linus Torvalds 2022-03-25 20:40 ` Borislav Petkov 2022-03-25 20:51 ` Linus Torvalds 2022-03-28 14:21 ` Jonathan Corbet 2022-03-28 15:56 ` Borislav Petkov 2022-03-25 21:07 ` pr-tracker-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).