Marking TSC as unstable has a side effect of marking sched_clock as
unstable when TSC is still being used as the sched_clock. This is not
desirable. Hyper-V ultimately uses a paravirtualized clock source that
provides a stable scheduler clock even on systems without TscInvariant
CPU capability. Hence, mark_tsc_unstable() call should be called _after_
scheduler clock has been changed to the paravirtualized clocksource. This
will prevent any unwanted manipulation of the sched_clock. Only TSC will
be correctly marked as unstable.
Signed-off-by: Ani Sinha <[email protected]>
---
arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 22f13343b5da..715458b7729a 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
- } else {
- mark_tsc_unstable("running on Hyper-V");
}
/*
@@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
/* Register Hyper-V specific clocksource */
hv_init_clocksource();
#endif
+ /* TSC should be marked as unstable only after Hyper-V
+ * clocksource has been initialized. This ensures that the
+ * stability of the sched_clock is not altered.
+ */
+ if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
+ mark_tsc_unstable("running on Hyper-V");
}
static bool __init ms_hyperv_x2apic_available(void)
--
2.25.1
On Tue, Jul 13, 2021 at 08:35:21AM +0530, Ani Sinha wrote:
> Marking TSC as unstable has a side effect of marking sched_clock as
> unstable when TSC is still being used as the sched_clock. This is not
> desirable. Hyper-V ultimately uses a paravirtualized clock source that
> provides a stable scheduler clock even on systems without TscInvariant
> CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> scheduler clock has been changed to the paravirtualized clocksource. This
> will prevent any unwanted manipulation of the sched_clock. Only TSC will
> be correctly marked as unstable.
Correct me if I'm wrong, what you're trying to address is that
sched_clock remains marked as unstable even after Linux has switched to
a stable clock source.
I think a better approach will be to mark the sched_clock as stable when
we switch to the paravirtualized clock source.
See hyperv_timer.c:hv_setup_sched_clock.
Wei.
>
> Signed-off-by: Ani Sinha <[email protected]>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 22f13343b5da..715458b7729a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
> if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> - } else {
> - mark_tsc_unstable("running on Hyper-V");
> }
>
> /*
> @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> /* Register Hyper-V specific clocksource */
> hv_init_clocksource();
> #endif
> + /* TSC should be marked as unstable only after Hyper-V
> + * clocksource has been initialized. This ensures that the
> + * stability of the sched_clock is not altered.
> + */
> + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
> + mark_tsc_unstable("running on Hyper-V");
> }
>
> static bool __init ms_hyperv_x2apic_available(void)
> --
> 2.25.1
>
On Tue, Jul 13, 2021 at 01:04:46PM +0000, Wei Liu wrote:
> On Tue, Jul 13, 2021 at 08:35:21AM +0530, Ani Sinha wrote:
> > Marking TSC as unstable has a side effect of marking sched_clock as
> > unstable when TSC is still being used as the sched_clock. This is not
> > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > provides a stable scheduler clock even on systems without TscInvariant
> > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > scheduler clock has been changed to the paravirtualized clocksource. This
> > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > be correctly marked as unstable.
>
> Correct me if I'm wrong, what you're trying to address is that
> sched_clock remains marked as unstable even after Linux has switched to
> a stable clock source.
>
> I think a better approach will be to mark the sched_clock as stable when
> we switch to the paravirtualized clock source.
No.. unstable->stable transitions are unsound. You get to switch to your
paravirt clock earlier.
On Tue, 13 Jul 2021, Peter Zijlstra wrote:
> On Tue, Jul 13, 2021 at 01:04:46PM +0000, Wei Liu wrote:
> > On Tue, Jul 13, 2021 at 08:35:21AM +0530, Ani Sinha wrote:
> > > Marking TSC as unstable has a side effect of marking sched_clock as
> > > unstable when TSC is still being used as the sched_clock. This is not
> > > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > > provides a stable scheduler clock even on systems without TscInvariant
> > > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > > scheduler clock has been changed to the paravirtualized clocksource. This
> > > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > > be correctly marked as unstable.
> >
> > Correct me if I'm wrong, what you're trying to address is that
> > sched_clock remains marked as unstable even after Linux has switched to
> > a stable clock source.
> >
> > I think a better approach will be to mark the sched_clock as stable when
> > we switch to the paravirtualized clock source.
>
> No.. unstable->stable transitions are unsound. You get to switch to your
> paravirt clock earlier.
>
I believe manipulating sched_clock was never the intention of the original
author who added the code to mark tsc as unstable on hyper-V:
commit 88c9281a9fba67636ab26c1fd6afbc78a632374f
Author: Vitaly Kuznetsov <[email protected]>
Date: Wed Aug 19 09:54:24 2015 -0700
x86/hyperv: Mark the Hyper-V TSC as unstable
The original author simply wanted to mark TSC as unstable on hyper-V
systems because of reasons the above commit log will describe. Sched clock
manipulation happened accidentally because from where the
mark_tsc_unstable() was being called. This patch simply fixes this.
Michael Kelly from Microsoft has tested this patch already.
--Ani
On Tue, Jul 13, 2021 at 09:01:06PM +0530, Ani Sinha wrote:
>
>
> On Tue, 13 Jul 2021, Peter Zijlstra wrote:
>
> > On Tue, Jul 13, 2021 at 01:04:46PM +0000, Wei Liu wrote:
> > > On Tue, Jul 13, 2021 at 08:35:21AM +0530, Ani Sinha wrote:
> > > > Marking TSC as unstable has a side effect of marking sched_clock as
> > > > unstable when TSC is still being used as the sched_clock. This is not
> > > > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > > > provides a stable scheduler clock even on systems without TscInvariant
> > > > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > > > scheduler clock has been changed to the paravirtualized clocksource. This
> > > > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > > > be correctly marked as unstable.
> > >
> > > Correct me if I'm wrong, what you're trying to address is that
> > > sched_clock remains marked as unstable even after Linux has switched to
> > > a stable clock source.
> > >
> > > I think a better approach will be to mark the sched_clock as stable when
> > > we switch to the paravirtualized clock source.
> >
> > No.. unstable->stable transitions are unsound. You get to switch to your
> > paravirt clock earlier.
> >
>
> I believe manipulating sched_clock was never the intention of the original
> author who added the code to mark tsc as unstable on hyper-V:
>
> commit 88c9281a9fba67636ab26c1fd6afbc78a632374f
> Author: Vitaly Kuznetsov <[email protected]>
> Date: Wed Aug 19 09:54:24 2015 -0700
>
> x86/hyperv: Mark the Hyper-V TSC as unstable
>
>
> The original author simply wanted to mark TSC as unstable on hyper-V
> systems because of reasons the above commit log will describe. Sched clock
> manipulation happened accidentally because from where the
> mark_tsc_unstable() was being called. This patch simply fixes this.
>
> Michael Kelly from Microsoft has tested this patch already.
OK. In light of Peter's comment and this one, I'm fine with this patch.
Michael, can you give an ack or review here?
Thanks,
Wei.
>
> --Ani
>
From: Ani Sinha <[email protected]> Sent: Monday, July 12, 2021 8:05 PM
>
> Marking TSC as unstable has a side effect of marking sched_clock as
> unstable when TSC is still being used as the sched_clock. This is not
> desirable. Hyper-V ultimately uses a paravirtualized clock source that
> provides a stable scheduler clock even on systems without TscInvariant
> CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> scheduler clock has been changed to the paravirtualized clocksource. This
> will prevent any unwanted manipulation of the sched_clock. Only TSC will
> be correctly marked as unstable.
>
> Signed-off-by: Ani Sinha <[email protected]>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 22f13343b5da..715458b7729a 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
> if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> - } else {
> - mark_tsc_unstable("running on Hyper-V");
> }
>
> /*
> @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> /* Register Hyper-V specific clocksource */
> hv_init_clocksource();
> #endif
> + /* TSC should be marked as unstable only after Hyper-V
> + * clocksource has been initialized. This ensures that the
> + * stability of the sched_clock is not altered.
> + */
For multi-line comments like the above, the first comment line
should just be "/*". So:
/*
* TSC should be marked as unstable only after Hyper-V
* clocksource has been initialized. This ensures that the
* stability of the sched_clock is not altered.
*/
> + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
> + mark_tsc_unstable("running on Hyper-V");
> }
>
> static bool __init ms_hyperv_x2apic_available(void)
> --
> 2.25.1
Modulo the comment format,
Reviewed-by: Michael Kelley <[email protected]>
Tested-by: Michael Kelley <[email protected]>
On Tue, Jul 13, 2021 at 05:25:59PM +0000, Michael Kelley wrote:
> From: Ani Sinha <[email protected]> Sent: Monday, July 12, 2021 8:05 PM
[...]
> > /*
> > @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> > /* Register Hyper-V specific clocksource */
> > hv_init_clocksource();
> > #endif
> > + /* TSC should be marked as unstable only after Hyper-V
> > + * clocksource has been initialized. This ensures that the
> > + * stability of the sched_clock is not altered.
> > + */
>
> For multi-line comments like the above, the first comment line
> should just be "/*". So:
>
> /*
> * TSC should be marked as unstable only after Hyper-V
> * clocksource has been initialized. This ensures that the
> * stability of the sched_clock is not altered.
> */
>
>
> > + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
> > + mark_tsc_unstable("running on Hyper-V");
> > }
> >
> > static bool __init ms_hyperv_x2apic_available(void)
> > --
> > 2.25.1
>
> Modulo the comment format,
>
I can fix this while committing. No need to resend.
> Reviewed-by: Michael Kelley <[email protected]>
> Tested-by: Michael Kelley <[email protected]>
Thanks Michael.
Wei.
On Tue, 13 Jul 2021, Michael Kelley wrote:
> From: Ani Sinha <[email protected]> Sent: Monday, July 12, 2021 8:05 PM
> >
> > Marking TSC as unstable has a side effect of marking sched_clock as
> > unstable when TSC is still being used as the sched_clock. This is not
> > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > provides a stable scheduler clock even on systems without TscInvariant
> > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > scheduler clock has been changed to the paravirtualized clocksource. This
> > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > be correctly marked as unstable.
> >
> > Signed-off-by: Ani Sinha <[email protected]>
> > ---
> > arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index 22f13343b5da..715458b7729a 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
> > if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> > wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> > setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > - } else {
> > - mark_tsc_unstable("running on Hyper-V");
> > }
> >
> > /*
> > @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> > /* Register Hyper-V specific clocksource */
> > hv_init_clocksource();
> > #endif
> > + /* TSC should be marked as unstable only after Hyper-V
> > + * clocksource has been initialized. This ensures that the
> > + * stability of the sched_clock is not altered.
> > + */
>
> For multi-line comments like the above, the first comment line
> should just be "/*". So:
Hmm, checkpatch.pl in kernel tree did not complain :
total: 0 errors, 0 warnings, 20 lines checked
0001-Hyper-V-fix-for-unwanted-manipulation-of-sched_clock.patch has no
obvious style problems and is ready for submission.
However, I do know from my experience of submitting Qemu patches last
year that this is a requirement imposed by the Qemu community as
checkpatch.pl in qemu tree would complain otherwise. I also took a peek at
the Qemu git history. It seems they imported this check from the kernel's
checkpatch.pl with this commit in Qemu tree:
commit 8c06fbdf36bf4d4d486116200248730887a4d7d6
Author: Peter Maydell <[email protected]>
Date: Fri Dec 14 13:30:48 2018 +0000
scripts/checkpatch.pl: Enforce multiline comment syntax
Which adds this rule:
+ # Block comments use /* on a line of its own
+ if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/
+ $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
+ WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
+ }
But in kernel there is no such rule. Hmm. strange!
>
> /*
> * TSC should be marked as unstable only after Hyper-V
> * clocksource has been initialized. This ensures that the
> * stability of the sched_clock is not altered.
> */
>
>
> > + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
> > + mark_tsc_unstable("running on Hyper-V");
> > }
> >
> > static bool __init ms_hyperv_x2apic_available(void)
> > --
> > 2.25.1
>
> Modulo the comment format,
>
> Reviewed-by: Michael Kelley <[email protected]>
> Tested-by: Michael Kelley <[email protected]>
>
From: Ani Sinha <[email protected]> Sent: Tuesday, July 13, 2021 10:49 AM
>
> On Tue, 13 Jul 2021, Michael Kelley wrote:
>
> > From: Ani Sinha <[email protected]> Sent: Monday, July 12, 2021 8:05 PM
> > >
> > > Marking TSC as unstable has a side effect of marking sched_clock as
> > > unstable when TSC is still being used as the sched_clock. This is not
> > > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > > provides a stable scheduler clock even on systems without TscInvariant
> > > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > > scheduler clock has been changed to the paravirtualized clocksource. This
> > > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > > be correctly marked as unstable.
> > >
> > > Signed-off-by: Ani Sinha <[email protected]>
> > > ---
> > > arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
> > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > > index 22f13343b5da..715458b7729a 100644
> > > --- a/arch/x86/kernel/cpu/mshyperv.c
> > > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > > @@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
> > > if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> > > wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> > > setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > > - } else {
> > > - mark_tsc_unstable("running on Hyper-V");
> > > }
> > >
> > > /*
> > > @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> > > /* Register Hyper-V specific clocksource */
> > > hv_init_clocksource();
> > > #endif
> > > + /* TSC should be marked as unstable only after Hyper-V
> > > + * clocksource has been initialized. This ensures that the
> > > + * stability of the sched_clock is not altered.
> > > + */
> >
> > For multi-line comments like the above, the first comment line
> > should just be "/*". So:
>
> Hmm, checkpatch.pl in kernel tree did not complain :
>
> total: 0 errors, 0 warnings, 20 lines checked
>
> 0001-Hyper-V-fix-for-unwanted-manipulation-of-sched_clock.patch has no
> obvious style problems and is ready for submission.
>
> However, I do know from my experience of submitting Qemu patches last
> year that this is a requirement imposed by the Qemu community as
> checkpatch.pl in qemu tree would complain otherwise. I also took a peek at
> the Qemu git history. It seems they imported this check from the kernel's
> checkpatch.pl with this commit in Qemu tree:
>
> commit 8c06fbdf36bf4d4d486116200248730887a4d7d6
> Author: Peter Maydell <[email protected]>
> Date: Fri Dec 14 13:30:48 2018 +0000
>
> scripts/checkpatch.pl: Enforce multiline comment syntax
>
> Which adds this rule:
>
> + # Block comments use /* on a line of its own
> + if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/
> + $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
> + WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
> + }
>
>
> But in kernel there is no such rule. Hmm. strange!
>
>
See section 8 of "Documentation/process/coding-style.rst" in a Linux kernel
source code tree. :-)
Michael
On Tue, 13 Jul 2021, Michael Kelley wrote:
> From: Ani Sinha <[email protected]> Sent: Tuesday, July 13, 2021 10:49 AM
> >
> > On Tue, 13 Jul 2021, Michael Kelley wrote:
> >
> > > From: Ani Sinha <[email protected]> Sent: Monday, July 12, 2021 8:05 PM
> > > >
> > > > Marking TSC as unstable has a side effect of marking sched_clock as
> > > > unstable when TSC is still being used as the sched_clock. This is not
> > > > desirable. Hyper-V ultimately uses a paravirtualized clock source that
> > > > provides a stable scheduler clock even on systems without TscInvariant
> > > > CPU capability. Hence, mark_tsc_unstable() call should be called _after_
> > > > scheduler clock has been changed to the paravirtualized clocksource. This
> > > > will prevent any unwanted manipulation of the sched_clock. Only TSC will
> > > > be correctly marked as unstable.
> > > >
> > > > Signed-off-by: Ani Sinha <[email protected]>
> > > > ---
> > > > arch/x86/kernel/cpu/mshyperv.c | 8 ++++++--
> > > > 1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > > > index 22f13343b5da..715458b7729a 100644
> > > > --- a/arch/x86/kernel/cpu/mshyperv.c
> > > > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > > > @@ -370,8 +370,6 @@ static void __init ms_hyperv_init_platform(void)
> > > > if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> > > > wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, 0x1);
> > > > setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > > > - } else {
> > > > - mark_tsc_unstable("running on Hyper-V");
> > > > }
> > > >
> > > > /*
> > > > @@ -432,6 +430,12 @@ static void __init ms_hyperv_init_platform(void)
> > > > /* Register Hyper-V specific clocksource */
> > > > hv_init_clocksource();
> > > > #endif
> > > > + /* TSC should be marked as unstable only after Hyper-V
> > > > + * clocksource has been initialized. This ensures that the
> > > > + * stability of the sched_clock is not altered.
> > > > + */
> > >
> > > For multi-line comments like the above, the first comment line
> > > should just be "/*". So:
> >
> > Hmm, checkpatch.pl in kernel tree did not complain :
> >
> > total: 0 errors, 0 warnings, 20 lines checked
> >
> > 0001-Hyper-V-fix-for-unwanted-manipulation-of-sched_clock.patch has no
> > obvious style problems and is ready for submission.
> >
> > However, I do know from my experience of submitting Qemu patches last
> > year that this is a requirement imposed by the Qemu community as
> > checkpatch.pl in qemu tree would complain otherwise. I also took a peek at
> > the Qemu git history. It seems they imported this check from the kernel's
> > checkpatch.pl with this commit in Qemu tree:
> >
> > commit 8c06fbdf36bf4d4d486116200248730887a4d7d6
> > Author: Peter Maydell <[email protected]>
> > Date: Fri Dec 14 13:30:48 2018 +0000
> >
> > scripts/checkpatch.pl: Enforce multiline comment syntax
> >
> > Which adds this rule:
> >
> > + # Block comments use /* on a line of its own
> > + if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/
> > + $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
> > + WARN("Block comments use a leading /* on a separate line\n" . $herecurr);
> > + }
> >
> >
> > But in kernel there is no such rule. Hmm. strange!
> >
> >
>
> See section 8 of "Documentation/process/coding-style.rst" in a Linux kernel
> source code tree. :-)
>
Yes that is fine. What I was confused about is that the commenting rule
existed in Qemu and checkpatch.pl there enforced this. Upon digging, I
found that the Qemu community themselves "imported" this formating rule
from kernel. Yet, kernel's own checkpatch.pl did not enforce this although
as you point above, the rule exists in written form.
This diff will fix kernel's checkpatch.pl without breaking drivers/net or
net/ . Enforcing rules through proper tooling saves everyone's time and
also ensures consistency.
Will send out this patch soon.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 23697a6b1eaa..5f047b762aa1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3833,6 +3833,14 @@ sub process {
"networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev);
}
+# Block comments use /* on a line of its own
+ if (!($realfile =~ m@^(drivers/net/|net/)@) &&
+ $rawline !~ m@^\+.*/\*.*\*/[ \t)}]*$@ && #inline /*...*/
+ $rawline =~ m@^\+.*/\*\*?+[ \t]*[^ \t]@) { # /* or /** non-blank
+ WARN("BLOCK_COMMENT_STYLE",
+ "Block comments use a leading /* on a separate line\n" . $herecurr);
+ }
+
# Block comments use * on subsequent lines
if ($prevline =~ /$;[ \t]*$/ && #ends in comment
$prevrawline =~ /^\+.*?\/\*/ && #starting /*