[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3] x86: correct socket_cpumask allocation



>>> On 10.07.15 at 17:13, <JBeulich@xxxxxxxx> wrote:
>>>> On 10.07.15 at 16:57, <dario.faggioli@xxxxxxxxxx> wrote:
>> I added this printk:
>> 
>> printk("  cpu=%u cpu_to_socket=%u\n", cpu, socket);
>> 
>> Right before this chunk of code, in cpu_smpboot_free():
>> 
>>     if ( cpumask_empty(socket_cpumask[socket]) )
>>     {
>>         xfree(socket_cpumask[socket]);
>>         socket_cpumask[socket] = NULL;
>>     }
>> 
>> And it says this:
>> 
>> ...
>> (XEN) Preparing system for ACPI S5 state.
>> (XEN) Disabling non-boot CPUs ...
>> (XEN) Broke affinity for irq 9
>> (XEN)   cpu=1 cpu_to_socket=4294967295
>> (XEN) ----[ Xen-4.6-unstable  x86_64  debug=y  Tainted:    C ]----
>> (XEN) CPU:    0
>> (XEN) RIP:    e008:[<ffff82d0801886c2>] cpu_smpboot_free+0x43/0x28b
>> ...
>> 
>> I.e., it looks like phys_proc_id has already been reset to
>> XEN_INVALID_SOCKET_ID, as we're kind-of racing with
>> remove_siblinginfo().
> 
> Right. We have
> 
> cpu_down()
>   stop_machine_run(take_cpu_down, ...)
>     notifier_call_chain(&cpu_chain, CPU_DYING, ...)
>     __cpu_disable()
>       remove_siblinginfo()
>   __cpu_die()
> notifier_call_chain(&cpu_chain, CPU_DEAD, ...)
>   cpu_smpboot_free()
> 
> I.e. a clear use-after-invalidate.

And I can't see a reason why we shouldn't be able to defer invoking
remove_siblinginfo() until cpu_smpboot_free(): Other than its
counterpart (set_cpu_sibling_map()) it doesn't require to be run on
the subject CPU (that function itself doesn't appear to depend on
that either, but it depends on identify_cpu() having run). Which
would at once allow reducing code: The clearing of
cpu_{core,sibling}_mask then becomes redundant with the freeing
of these masks.

Of course there may be hidden dependencies, so maybe a safer
approach would be to just move the zapping of the three IDs
(and maybe the clearing of the CPU's cpu_sibling_setup_map bit)
into cpu_smpboot_free().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.