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

[Peter Zijlstra <peterz@xxxxxxxxxxxxx>]

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);
> +}