Re: [PATCH 1/4] x86: move syscall trampolines off the stack

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 20/05/15 14:37, Jan Beulich wrote:
>>>> On 19.05.15 at 18:59, <andrew.cooper3@citrix.com> wrote:
>> On 18/05/15 13:46, Jan Beulich wrote:
>>> @@ -616,6 +653,24 @@ static void cpu_smpboot_free(unsigned in
>>>      free_cpumask_var(per_cpu(cpu_sibling_mask, cpu));
>>>      free_cpumask_var(per_cpu(cpu_core_mask, cpu));
>>>  
>>> +    if ( per_cpu(stubs.addr, cpu) )
>>> +    {
>>> +        unsigned long mfn = per_cpu(stubs.mfn, cpu);
>>> +        unsigned char *stub_page = map_domain_page(mfn);
>>> +        unsigned int i;
>>> +
>>> +        memset(stub_page + (cpu & (STUBS_PER_PAGE - 1)) * STUB_BUF_SIZE,
>>> +               0xcc, STUB_BUF_SIZE);
>>> +        for ( i = 0; i < STUBS_PER_PAGE; ++i )
>>> +            if ( stub_page[i * STUB_BUF_SIZE] != 0xcc)
>>> +                break;
>> There is a race condition between allocate and free, as
>> write_stub_trampoline() is written by the cpu itself.  Just finding 0xcc
>> here doesn't mean there isn't a different cpu in the process of coming
>> up and intending to use the stub page.
> No, there is no race afaict: percpu_traps_init() gets called before
> a CPU is marked online and hence before __cpu_up() returns, and
> thus before the CPU hotplug lock gets dropped, so there can't be
> any CPUs going down concurrently.

Thinking out loud...

cpu_up/down have excusion due to the recursive spinlock used by
cpu_hotplug_begin/end.

The allocate is done by the CPU_UP_PREPARE notifier chain, and
__cpu_up() does wait until the AP has marked itself as online.

Therefore, so long as no CPU_UP_PREPARE action triggers a
CPU_UP_CANCELED or CPU_DEAD chain against the same cpu, we are safe.

~Andrew