Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

Peter Zijlstra
2023-05-09T10:20:06Z

Again, not really this patch, but since I had to look at this code ....

On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
> @@ -1048,60 +1066,89 @@ static int do_boot_cpu(int apicid, int c

/*
* AP might wait on cpu_callout_mask in cpu_init() with
* cpu_initialized_mask set if previous attempt to online
* it timed-out. Clear cpu_initialized_mask so that after
* INIT/SIPI it could start with a clean state.
*/
cpumask_clear_cpu(cpu, cpu_initialized_mask);
smp_mb();

^^^ that barrier is weird too, cpumask_clear_cpu() is an atomic op and
implies much the same (this is x86 after all). If you want to be super
explicit about it write:

smp_mb__after_atomic();

(which is a no-op) but then it still very much requires a comment as to
what exactly it orders against what.

/*
* Wake up a CPU in difference cases:
* - Use a method from the APIC driver if one defined, with wakeup
* straight to 64-bit mode preferred over wakeup to RM.
* Otherwise,
> * - Use an INIT boot APIC message
> */
> if (apic->wakeup_secondary_cpu_64)
> + return apic->wakeup_secondary_cpu_64(apicid, start_ip);
> else if (apic->wakeup_secondary_cpu)
> + return apic->wakeup_secondary_cpu(apicid, start_ip);
>
> + return wakeup_secondary_cpu_via_init(apicid, start_ip);
> +}