Re: [Xen-devel] [PATCH v2] add locking around certain calls to map_pages_to_xen()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.07.13 at 14:15, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> On 12/07/2013 09:17, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> While boot time calls don't need this, run time uses of the function
>> which may result in L2 page tables getting populated need to be
>> serialized to avoid two CPUs populating the same L2 (or L3) entry,
>> overwriting each other's results.
>> 
>> This fixes what would seem to be a regression from commit b0581b92
>> ("x86: make map_domain_page_global() a simple wrapper around vmap()"),
>> albeit that change only made more readily visible the already existing
>> issue.
>> 
>> The __init addition to memguard_init(), while seemingly unrelated,
>> helps making obvious that this function's use of map_pages_to_xen() is
>> a boot time only one.
> 
> Why can't the locking be implemented inside map_pages_to_xen()? The
> requirement is due to implementation details of that function after all.
> Pushing the synchronisation out to the callers is uglier and more fragile.

Not all use cases of the function require synchronization, so the
only kind of synchronization I would see validly adding there
instead of in the callers would be a mechanism marking a to-be-
populated non-leaf page table entry as "being processed" first,
and have racing invocations spin until that state clears. Afaict
that wouldn't cope with eventual (future) races through
destroy_xen_mappings() though, and hence I think the proposed
patch is the better alternative. But if you're fine with ignoring
that last aspect, I'm okay with going the alternative route.

Jan


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