Linux-Sparse Archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Guenter Roeck <linux@roeck-us.net>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Linus Torvalds <torvalds@linuxfoundation.org>,
	Uros Bizjak <ubizjak@gmail.com>,
	linux-sparse@vger.kernel.org, lkp@intel.com,
	oe-kbuild-all@lists.linux.dev
Subject: Re: [patch 5/9] x86: Cure per CPU madness on UP
Date: Thu, 21 Mar 2024 17:49:30 +0100	[thread overview]
Message-ID: <87il1fh6n9.ffs@tglx> (raw)
In-Reply-To: <86796005-f1d9-4c8c-80d8-f1f88ca220ba@roeck-us.net>

On Thu, Mar 21 2024 at 07:06, Guenter Roeck wrote:
> On 3/21/24 04:14, Thomas Gleixner wrote:
>> If so can you please provide a full dmesg and then apply the patch below
>> and provide the resulting full dmesg too?
>
> You'll find everything at http://server.roeck-us.net/qemu/x86-nosmp/

Thanks for providing this.

> The crash is gone after applying your patch. The difference is:
>
> +       /*
> +        * If there was no APIC registered, then the map check below would
> +        * fail. With no APIC this is guaranteed to be an UP system and
> +        * therefore all topology levels have only one entry and their
> +        * logical ID is obviously 0.
> +        */
> +       if (topo_info.boot_cpu_apic_id == BAD_APICID) {
> +               pr_info("#### topo_info.boot_cpu_apic_id == BAD_APICID\n");
>                  ^^^^ I added this
> +               return 0;
> +       }
> +
>
> I see the "#### topo_info.boot_cpu_apic_id == BAD_APICID" message
> twice in the log. See patched.log at the page pointed to above.

I can see why this is emitted. That happens on the initial CPUID
evaluation of the boot CPU very early during boot.

[    0.000000] Command line: console=ttyS0
[    0.000000] CPU topo: APIC logical ID: 0 0 6
[    0.000000] CPU topo: #### topo_info.boot_cpu_apic_id == BAD_APICID
[    0.000000] CPU topo: APIC logical ID: 0 0 4
[    0.000000] CPU topo: #### topo_info.boot_cpu_apic_id == BAD_APICID

The later full CPUID evaluation happens after the ACPI enumeration and
way before the affected RAPL driver is initialized:

[    0.088029] CPU topo: APIC logical ID: 0 0 6
[    0.088084] CPU topo: APIC logical ID: 0 0 4

This invocation has the boot APIC registered as your extra print does
not show up.

...

[    0.585850] RAPL PMU: API unit is 2^-32 Joules, 0 fixed counters, 10737418240 ms ovfl timer

So even without that guard (which we need anyway for the non APIC case)
topology_logical_die_id() == cpu_data(cpu).topo.logical_die_id must have
the correct value in that RAPL initialization and CPU hotplug callback
code.

But our absolutely convoluted startup logic prevents that because:

    identify_cpu_early()      operates on boot_cpu_data
    smp_prepare_boot_cpu()    copies boot_cpu_data to per CPU cpu data
    identify_boot_cpu()       operates on boot_cpu_data

identify_boot_cpu() is the one which gets the correct logical die info,
but that never gets copied over to the per CPU data instance on which
the RAPL code and everything else works on.

I'll cook up a patch later.

Thanks for providing the info!

       tglx


  reply	other threads:[~2024-03-21 16:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-04 10:12 [patch 0/9] x86: Cure tons of sparse warnings (mostly __percpu) Thomas Gleixner
2024-03-04 10:12 ` [patch 1/9] perf/x86/amd/uncore: Fix __percpu annotation Thomas Gleixner
2024-03-04 10:12 ` [patch 2/9] x86/msr: Prepare for including percpu.h Thomas Gleixner
2024-03-04 10:12 ` [patch 3/9] x86/msr: Add missing __percpu annotations Thomas Gleixner
2024-03-04 10:12 ` [patch 4/9] smp: Consolidate smp_prepare_boot_cpu() Thomas Gleixner
2024-03-04 10:12 ` [patch 5/9] x86: Cure per CPU madness on UP Thomas Gleixner
2024-03-15 16:17   ` Guenter Roeck
2024-03-15 16:42     ` Linus Torvalds
2024-03-15 17:02       ` Guenter Roeck
2024-03-15 17:40       ` Thomas Gleixner
2024-03-15 22:55         ` Thomas Gleixner
2024-03-15 23:23           ` Linus Torvalds
2024-03-16  1:11             ` Thomas Gleixner
2024-03-16  1:23               ` Linus Torvalds
2024-03-16 21:34                 ` Arnd Bergmann
2024-03-17 21:03               ` David Laight
2024-03-18 11:11               ` Thomas Gleixner
2024-03-18 17:27               ` Thomas Gleixner
2024-03-18 19:13                 ` Arnd Bergmann
2024-03-19 16:21                   ` Thomas Gleixner
2024-03-19 18:26                     ` Guenter Roeck
2024-03-16  0:56           ` Guenter Roeck
2024-03-20  8:58     ` Thomas Gleixner
2024-03-20 15:46       ` Guenter Roeck
2024-03-21 11:14         ` Thomas Gleixner
2024-03-21 14:06           ` Guenter Roeck
2024-03-21 16:49             ` Thomas Gleixner [this message]
2024-03-04 10:12 ` [patch 6/9] x86/uaccess: Add missing __force to casts in __access_ok() and valid_user_address() Thomas Gleixner
2024-03-04 10:12 ` [patch 7/9] x86/cpu: Use EXPORT_PER_CPU_SYMBOL_GPL() for x86_spec_ctrl_current Thomas Gleixner
2024-03-04 10:12 ` [patch 8/9] x86/cpu: Provide a declaration for itlb_multihit_kvm_mitigation Thomas Gleixner
2024-03-04 10:12 ` [patch 9/9] x86/callthunks: Use EXPORT_PER_CPU_SYMBOL_GPL() for per CPU variables Thomas Gleixner
2024-03-04 11:08 ` [patch 0/9] x86: Cure tons of sparse warnings (mostly __percpu) Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87il1fh6n9.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sparse@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=torvalds@linuxfoundation.org \
    --cc=ubizjak@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).