Use u32 instead of uint32_t in hypervisor_cpuid_base().
Yes, I realize uint32_t is used in Xen code et al, but this is
a core x86 architecture header and we should standardize on the
type that is being used overwhelmingly in related x86 architecture
code.
The two types are the same so there should be no build warnings.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Ahmed S. Darwish <darwi@linutronix.de>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: x86-cpuid@lists.linux.dev
Link: https://lore.kernel.org/r/20250317164745.4754-3-darwi@linutronix.de
---
arch/x86/include/asm/cpuid/api.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
index 356db1894588..9c180c9cc58e 100644
--- a/arch/x86/include/asm/cpuid/api.h
+++ b/arch/x86/include/asm/cpuid/api.h
@@ -187,9 +187,9 @@ static __always_inline bool cpuid_function_is_indexed(u32 function)
#define for_each_possible_hypervisor_cpuid_base(function) \
for (function = 0x40000000; function < 0x40010000; function += 0x100)
-static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
+static inline u32 hypervisor_cpuid_base(const char *sig, u32 leaves)
{
- uint32_t base, eax, signature[3];
+ u32 base, eax, signature[3];
for_each_possible_hypervisor_cpuid_base(base) {
cpuid(base, &eax, &signature[0], &signature[1], &signature[2]);
--
2.45.2On 3/17/2025 3:18 PM, Ingo Molnar wrote:
> Use u32 instead of uint32_t in hypervisor_cpuid_base().
>
> Yes, I realize uint32_t is used in Xen code et al, but this is
> a core x86 architecture header and we should standardize on the
no "we", right?
> type that is being used overwhelmingly in related x86 architecture
> code.
>
> The two types are the same so there should be no build warnings.
>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Ahmed S. Darwish <darwi@linutronix.de>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: x86-cpuid@lists.linux.dev
> Link: https://lore.kernel.org/r/20250317164745.4754-3-darwi@linutronix.de
> ---
> arch/x86/include/asm/cpuid/api.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
> index 356db1894588..9c180c9cc58e 100644
> --- a/arch/x86/include/asm/cpuid/api.h
> +++ b/arch/x86/include/asm/cpuid/api.h
> @@ -187,9 +187,9 @@ static __always_inline bool cpuid_function_is_indexed(u32 function)
> #define for_each_possible_hypervisor_cpuid_base(function) \
> for (function = 0x40000000; function < 0x40010000; function += 0x100)
>
> -static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
> +static inline u32 hypervisor_cpuid_base(const char *sig, u32 leaves)
> {
> - uint32_t base, eax, signature[3];
> + u32 base, eax, signature[3];
>
> for_each_possible_hypervisor_cpuid_base(base) {
> cpuid(base, &eax, &signature[0], &signature[1], &signature[2]);
* Xin Li <xin@zytor.com> wrote:
> On 3/17/2025 3:18 PM, Ingo Molnar wrote:
> > Use u32 instead of uint32_t in hypervisor_cpuid_base().
> >
> > Yes, I realize uint32_t is used in Xen code et al, but this is
> > a core x86 architecture header and we should standardize on the
>
> no "we", right?
That's a stupid rule, I don't know where it came from, and I never
enforced it. It's not in Documentation/process/coding-style.rst.
Linus doesn't use this pointless rule of 'pronoun avoidance' in
changelogs either:
00a7d39898c8 ("fs/pipe: add simpler helpers for common cases")
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=00a7d39898c8010bfd5ff62af31ca5db34421b38
It turns out that we don't have _that_ many places that access these
^^
fields directly and were affected, but we have more than we strictly
^^ ^^
should have, because our low-level helper functions have been designed
to have intimate knowledge of how the pipes work.
And as a result, that random noise of direct 'pipe->head' and
'pipe->tail' accesses makes it harder to pinpoint any actual potential
problem spots remaining.
For example, we didn't have a "is the pipe full" helper function, but
^^
instead had a "given these pipe buffer indexes and this pipe size, is
the pipe full". That's because some low-level pipe code does actually
want that much more complicated interface.
In changelogs 'we' when used as a generic personal pronoun means the
kernel and the kernel community in general. It's a perfectly fine
grammatical construct.
Thanks,
Ingo
On Tue, Mar 18, 2025 at 09:34:41AM +0100, Ingo Molnar wrote:
> That's a stupid rule, I don't know where it came from, and I never
> enforced it. It's not in Documentation/process/coding-style.rst.
I believe tglx came up with it - section "Changelog" in
Documentation/process/maintainer-tip.rst
Read the examples there.
And you and I have had this conversation already on IRC. I happen to agree
with him that "we" is ambiguous - with all those companies submitting patches
you don't know who's "we" interests are being taken care of.
And if you formulate your commit message in impersonal tone, it reads a lot
clearer. It is simply a lot better this way.
Is it a hard rule? Ofc not - there are exceptions to that rule depending on
the context. But most if the time and IMNSVHO, impersonal formulations read
a lot better and clearer.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
* Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Mar 18, 2025 at 09:34:41AM +0100, Ingo Molnar wrote:
> > That's a stupid rule, I don't know where it came from, and I never
> > enforced it. It's not in Documentation/process/coding-style.rst.
>
> I believe tglx came up with it - section "Changelog" in
>
> Documentation/process/maintainer-tip.rst
>
> Read the examples there.
Literally the first example there is kinda bogus:
Example 1::
...
When a CPU is dying, we cancel the worker and schedule a new worker
on a different CPU on the same domain.
Improved version::
...
When a CPU is dying, the worker is canceled and a new worker is
scheduled on a different CPU in the same domain.
[ Note that I edited the first example to be a true equivalent
transformation to passive voice. The example in maintainer-tip.rst
makes other edits too which make it hard to compare. ]
How is one more word and saying the same thing in a more circumspect
fashion a liguistic improvement?
And you don't have to believe me - I gave an LLM the following prompt:
Which English sentence is easier to understand:
"When a CPU is dying, the worker is canceled and a new worker is
scheduled on a different CPU in the same domain."
or
"When a CPU is dying, we cancel the worker and schedule a new worker
on a different CPU on the same domain."?
And it answered:
The second sentence, "When a CPU is dying, we cancel the worker and
schedule a new worker on a different CPU on the same domain," is easier
to understand. It uses simpler language and a more direct structure,
making it clearer for the reader.
... and although I'd be the first one to distrust an LLM's opinion,
it's correct in this case IMHO.
> And you and I have had this conversation already on IRC. I happen to
> agree with him that "we" is ambiguous - with all those companies
> submitting patches you don't know who's "we" interests are being
> taken care of.
Few people will understand a generic personal pronoun to apply to a
corporate entity magically, unless it's really clear and specific:
"We at Intel believe that this condition cannot occur on Intel
hardware."
in which case it's not a generic personal pronoun anymore.
Or to give another data point: since the v6.13 merge cycle we have
merged over 11,000 commits in the upstream kernel, and over 1,500
contain the word 'we' - over 13% of all commits. This is literally a
pointless battle that creates unnecessary maintenance overhead and
pointless detours for developers.
> And if you formulate your commit message in impersonal tone, it reads a lot
> clearer. It is simply a lot better this way.
Except *not even we* follow it consistently:
starship:~/tip> gl --author=tglx --since=two-years-ago --grep='\<we\>' linus | grep -iw we
by a context from task B and we do the check
So it turns out that we have to do two passes of
"The problem in current microcode loading method is that we load a
microcode way, way too late; ideally we should load it before turning
paging on. This may only be practical on 32 bits since we can't get
to 64-bit mode without paging on, but we should still do it as early
MADT delivers we only trust the hardware anyway.
* booting is too fragile that we want to limit the
Because it's actually a natural and direct linguistic construct.
And have a look at:
$ gl --author=torvalds --since=two-years-ago --grep='\<we\>' linus | grep -iw we
it's 1352 examples of Linus using 'we' as a generic personal pronoun in
the last 2 years alone...
Thanks,
Ingo
On Tue, Mar 18, 2025 at 12:53:05PM +0100, Ingo Molnar wrote:
> How is one more word and saying the same thing in a more circumspect
> fashion a liguistic improvement?
Because it removes the "we" out of the equation. I don't have to wonder who's
the "we" the author is talking about: his employer, his private interests in
Linux or "we" is actually "us" - the community as a whole.
I can't give a more honking example about the ambiguity here.
> The second sentence, "When a CPU is dying, we cancel the worker and
> schedule a new worker on a different CPU on the same domain," is easier
> to understand. It uses simpler language and a more direct structure,
> making it clearer for the reader.
I disagree with the LLM - it is yet another proof that AI won't replace
humans - if anything it'll make them *think* more. Which is good! :-)
> Few people will understand a generic personal pronoun to apply to a
> corporate entity magically, unless it's really clear and specific:
>
> "We at Intel believe that this condition cannot occur on Intel
> hardware."
>
> in which case it's not a generic personal pronoun anymore.
Except no one says "we at <company>" - they say "we" ambiguously. And I have
had gazillion examples of "we the company want Linux to do this and that
because our use case is bla".
> Or to give another data point: since the v6.13 merge cycle we have
<snip the stats>
That's why I said
"Is it a hard rule? Ofc not - there are exceptions to that rule depending on
the context."
And we have said "we" for 30+ years so can't change that over night. And not
everyone agrees with that. I understand it all.
I still think that in some cases formulating a commit message in impersonal
style lets you concentrate on the *problem* at hand the commit is trying to
fix - not what we do or want. It removes the person out of the equation
because the person doesn't need to be there.
HOWEVER, it is perfectly fine to say "I did this and that and I've been
wondering for years why the code does what it does." because it adds that
additional coloring about the trials and tribulations of the author.
So no, it is not a hard rule but there is an undeniable merit in writing the
commit messages impersonal.
And that's fine - I fix up things from time to time when they bother me.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
* Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Mar 18, 2025 at 12:53:05PM +0100, Ingo Molnar wrote:
> > How is one more word and saying the same thing in a more circumspect
> > fashion a liguistic improvement?
>
> Because it removes the "we" out of the equation. I don't have to
> wonder who's the "we" the author is talking about: his employer, his
> private interests in Linux or "we" is actually "us" - the community
> as a whole.
In practice this is almost never ambiguous - and when it is, it can be
fixed up.
> I can't give a more honking example about the ambiguity here.
It's a red herring fallacy really. Let's go over the first example
given in Documentation/process/maintainer-tip.rst:
x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu
When a CPU is dying, we cancel the worker and schedule a new worker on a
different CPU on the same domain. But if the timer is already about to
expire (say 0.99s) then we essentially double the interval.
You'd have to be a bumbling idiot to think that the 'we' means an
employer or the person themselves ...
Put differently: *the very first example given* uses 'we' functionally
unambiguously so that everyone who can read kernel changelogs will
understand what it says. Ie. the whole policy is based on a false
statement...
Very few of the 'we' general pronouns used in kernel changelogs are
actually ambiguous. This means that any crusade to eliminate 'we' from
changelogs is not just pointless, but also a waste of resources - it's
a net negative. At least IMHO. ;-)
> > The second sentence, "When a CPU is dying, we cancel the worker
> > and schedule a new worker on a different CPU on the same domain,"
> > is easier to understand. It uses simpler language and a more
> > direct structure, making it clearer for the reader.
>
> I disagree with the LLM - it is yet another proof that AI won't
> replace humans - if anything it'll make them *think* more. Which is
> good! :-)
Yeah, and in any case, tastes differ, so no strong feelings from me
either! :-)
Thanks,
Ingo
On Tue, Mar 18 2025 at 19:20, Ingo Molnar wrote:
> * Borislav Petkov <bp@alien8.de> wrote:
>> On Tue, Mar 18, 2025 at 12:53:05PM +0100, Ingo Molnar wrote:
>> > How is one more word and saying the same thing in a more circumspect
>> > fashion a liguistic improvement?
>>
>> Because it removes the "we" out of the equation. I don't have to
>> wonder who's the "we" the author is talking about: his employer, his
>> private interests in Linux or "we" is actually "us" - the community
>> as a whole.
>
> In practice this is almost never ambiguous - and when it is, it can be
> fixed up.
>
>> I can't give a more honking example about the ambiguity here.
>
> It's a red herring fallacy really. Let's go over the first example
> given in Documentation/process/maintainer-tip.rst:
>
> x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu
>
> When a CPU is dying, we cancel the worker and schedule a new worker on a
> different CPU on the same domain. But if the timer is already about to
> expire (say 0.99s) then we essentially double the interval.
>
> You'd have to be a bumbling idiot to think that the 'we' means an
> employer or the person themselves ...
>
> Put differently: *the very first example given* uses 'we' functionally
> unambiguously so that everyone who can read kernel changelogs will
> understand what it says. Ie. the whole policy is based on a false
> statement...
That's complete and utter nonsense.
'we cancel the worker, we call kmalloc()' are purely colloquial
expressions. Liguistically they are factually wrong abominations.
We can cancel a subscription, an appointment, a booking...
We can call a taxi, a ambulance, a doctor, ....
But as a matter of fact, we _cannot_ cancel a worker or call kmalloc().
Changelogs as any other serious writing in technical context are about
precision and clarity.
The impersonating form is obviously popular and in some contexts, like
tutorials and beginner guides, it makes them seemingly more accessible,
but that does not provide an justification for using it in the context
of change logs.
Change logs are an important documentation of the underlying code
change, because they provide context and technical justification for the
change and therefore have to prioritize precision and clarity.
Aside of that ,writing a change log in neutral and technically precise
language forces you to actually rethink the problem and the approach to
solve it. Dumping your half baked thoughts in impersonating novel style
does not.
From 20+ years of experience on the receiving end of the patch fire
hose, I can clearly proof a very high correlation between the quality of
change logs and the quality of the analysis and the resulting code
change.
Yes, it is work to write a proper and precise change log, but that extra
effort makes the work of people, who review patches, easier and it's
also highly benefitial, when analyising historical changes.
Thanks,
tglx
* Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, Mar 18 2025 at 19:20, Ingo Molnar wrote:
> > * Borislav Petkov <bp@alien8.de> wrote:
> >> On Tue, Mar 18, 2025 at 12:53:05PM +0100, Ingo Molnar wrote:
> >> > How is one more word and saying the same thing in a more circumspect
> >> > fashion a liguistic improvement?
> >>
> >> Because it removes the "we" out of the equation. I don't have to
> >> wonder who's the "we" the author is talking about: his employer, his
> >> private interests in Linux or "we" is actually "us" - the community
> >> as a whole.
> >
> > In practice this is almost never ambiguous - and when it is, it can be
> > fixed up.
> >
> >> I can't give a more honking example about the ambiguity here.
> >
> > It's a red herring fallacy really. Let's go over the first example
> > given in Documentation/process/maintainer-tip.rst:
> >
> > x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu
> >
> > When a CPU is dying, we cancel the worker and schedule a new worker on a
> > different CPU on the same domain. But if the timer is already about to
> > expire (say 0.99s) then we essentially double the interval.
> >
> > You'd have to be a bumbling idiot to think that the 'we' means an
> > employer or the person themselves ...
> >
> > Put differently: *the very first example given* uses 'we' functionally
> > unambiguously so that everyone who can read kernel changelogs will
> > understand what it says. Ie. the whole policy is based on a false
> > statement...
>
> That's complete and utter nonsense.
I love you too! :-)
> 'we cancel the worker, we call kmalloc()' are purely colloquial
> expressions.
So what? I have no problem with colloquial, familiar, everyday language
in a technical context as long as it's effective and unambiguous.
The main linguistic advantage of German engineering is the ability to
construct new, unambiguous words out of thin air:
"Donaudampfschifffahrtselektrizitätenhauptbetriebswerkbauunternehmenbeamtengesellschaft"
... not the cold, impersonal tone. And I say that as a German, and yes,
the 87-letter word above is a real, valid German word. :-)
> Liguistically they are factually wrong abominations.
>
> We can cancel a subscription, an appointment, a booking... We can
> call a taxi, a ambulance, a doctor, ....
>
> But as a matter of fact, we _cannot_ cancel a worker or call
> kmalloc().
Nor can we read a source buffer, nor can we do multiple writes to a
destination buffer, right?
Tell that to Linus, who arguably writes one of the best changelogs in
the kernel:
# 9022ed0e7e65 ("strscpy: write destination buffer only once")
In particular, the same way we shouldn't read the source buffer more
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
than once, we should avoid doing multiple writes to the destination
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
buffer: first writing a potentially non-terminated string, and then
^^^^^^^
terminating it with NUL at the end does not result in a stable result
buffer.
And I think the moment you have to argue against the quality of Linus's
changelogs you've lost the argument really, almost by default.
> Changelogs as any other serious writing in technical context are about
> precision and clarity.
Absolutely, and 'we' in this context unambiguously means the kernel, so
it's as clear to me as it gets.
I (obviously) agree with most of the stylistic and linguistic
suggestions in Documentation/process/maintainer-tip.rst, and maybe my
reaction was a bit hyperbolic (sorry), I just pointed out that this
silly avoidance of pronouns like 'we' - which started the discussion -
which results in *sentences with more words*, is *obviously*
counterproductive.
Longer sentences with the same information content == worse.
To visualize it:
When a CPU is dying, the worker is canceled and a new worker is scheduled on a different CPU in the same domain.
When a CPU is dying, we cancel the worker and schedule a new worker on a different CPU in the same domain.
In communication shorter is better, if the information content is
otherwise equivalent.
Anyway, let's agree to disagree. :-)
Thanks,
Ingo
The following commit has been merged into the x86/core branch of tip:
Commit-ID: a46f322661857d58b93f557bcb708260f18c18fd
Gitweb: https://git.kernel.org/tip/a46f322661857d58b93f557bcb708260f18c18fd
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Mon, 17 Mar 2025 23:18:24 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 19 Mar 2025 11:19:28 +01:00
x86/cpuid: Use u32 in instead of uint32_t in <asm/cpuid/api.h>
Use u32 instead of uint32_t in hypervisor_cpuid_base().
Yes, uint32_t is used in Xen code et al, but this is a core x86
architecture header and we should standardize on the type that
is being used overwhelmingly in related x86 architecture code.
The two types are the same so there should be no build warnings.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: "Ahmed S. Darwish" <darwi@linutronix.de>
Cc: x86-cpuid@lists.linux.dev
Link: https://lore.kernel.org/r/20250317221824.3738853-6-mingo@kernel.org
---
arch/x86/include/asm/cpuid/api.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
index 356db18..9c180c9 100644
--- a/arch/x86/include/asm/cpuid/api.h
+++ b/arch/x86/include/asm/cpuid/api.h
@@ -187,9 +187,9 @@ static __always_inline bool cpuid_function_is_indexed(u32 function)
#define for_each_possible_hypervisor_cpuid_base(function) \
for (function = 0x40000000; function < 0x40010000; function += 0x100)
-static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
+static inline u32 hypervisor_cpuid_base(const char *sig, u32 leaves)
{
- uint32_t base, eax, signature[3];
+ u32 base, eax, signature[3];
for_each_possible_hypervisor_cpuid_base(base) {
cpuid(base, &eax, &signature[0], &signature[1], &signature[2]);The following commit has been merged into the x86/cpu branch of tip:
Commit-ID: ba501f14e1e6dcc94ff0276301e997ae28e3f4b3
Gitweb: https://git.kernel.org/tip/ba501f14e1e6dcc94ff0276301e997ae28e3f4b3
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Mon, 17 Mar 2025 23:18:24 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 18 Mar 2025 09:35:58 +01:00
x86/cpuid: Use u32 in instead of uint32_t in <asm/cpuid/api.h>
Use u32 instead of uint32_t in hypervisor_cpuid_base().
Yes, uint32_t is used in Xen code et al, but this is a core x86
architecture header and we should standardize on the type that
is being used overwhelmingly in related x86 architecture code.
The two types are the same so there should be no build warnings.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: "Ahmed S. Darwish" <darwi@linutronix.de>
Cc: x86-cpuid@lists.linux.dev
Link: https://lore.kernel.org/r/20250317221824.3738853-6-mingo@kernel.org
---
arch/x86/include/asm/cpuid/api.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/cpuid/api.h b/arch/x86/include/asm/cpuid/api.h
index 356db18..9c180c9 100644
--- a/arch/x86/include/asm/cpuid/api.h
+++ b/arch/x86/include/asm/cpuid/api.h
@@ -187,9 +187,9 @@ static __always_inline bool cpuid_function_is_indexed(u32 function)
#define for_each_possible_hypervisor_cpuid_base(function) \
for (function = 0x40000000; function < 0x40010000; function += 0x100)
-static inline uint32_t hypervisor_cpuid_base(const char *sig, uint32_t leaves)
+static inline u32 hypervisor_cpuid_base(const char *sig, u32 leaves)
{
- uint32_t base, eax, signature[3];
+ u32 base, eax, signature[3];
for_each_possible_hypervisor_cpuid_base(base) {
cpuid(base, &eax, &signature[0], &signature[1], &signature[2]);© 2016 - 2025 Red Hat, Inc.