>-----Original Message-----
>From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>[mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Keir Fraser
>Sent: Tuesday, June 01, 2010 5:19 PM
>To: Liu, Jinsong; xen-devel@xxxxxxxxxxxxxxxxxxx
>Subject: [Xen-devel] Re: [PATCH] Fix cpu hotplug bug of mtrr update
>inconsistency
>
>On 01/06/2010 09:22, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
>
>> Fix cpu hotplug bug of mtrr update inconsistency
>>
>> c/s 20021 changes some mtrr update logic.
>> A bug is, when a cpu hot-add and then mtrr update, another cpu hot-add may
>> break cpu_online_map
>> consistency and result in deadlock.
>> This patch is used to fix the bug. It move 'mtrr_ap_init' back to bp-ap sync
>> stage protected by
>> CPU_STATE_CALLOUT and CPU_STATE_CALLIN, and then keep consistency.
>
>This is an old bug, rather than introduced by my changes, right?
Yes, not caused by the new changes.
>
>I suggest we leave the call where it is, and fix set_mtrr() to not race CPUs
>coming online. It is called elsewhere other than mtrr_ap_init() after all.
>Also if you call mtrr_ap_init() before being in cpu_online_map, you then
>race further MTRR changes:
There is a cpu_hotplug lock held when mtrr_add/del_page, but that lock is NULL
now.
One argument is, because the mtrr_add/del_page and the cpu_o*ling request all
from dom0, if dom0 should sync all vCPUs before mtrr_a/d_page, it should be
safe that there is no race. But not sure about dom0's operation.
--jyh
> - CPU X calls mtrr_ap_init()
> - CPU Y calls set_mtrr() to actually change an MTRR.
> - CPU X adds itself to cpu_online_map
> - Aiee, CPU X is missing Y's update
>
>I'll make a patch.
>
>By the way, our MTRR subsystem is really pants! :-/
>
> -- Keir
>
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@xxxxxxxxxxxxxxxxxxx
>http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|