Re: [Xen-devel] [PATCH 8 of 9] Modify all internal p2m functions to use the new fine-grained locking

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

The unlocked's are a shorthand. I might consider removing them if they
cause to much confusion.

In the case you're worried about, below, note there is a regular
gfn_to_mfn_query (which therefore locks the p2m entry) before the
drop_p2m. The previous scan uses unlocked -- we can forego unlocked there,
and keep the p2m entry locked the whole way. But then we need to make sure
we don't overflow recursive counters, since locks covers a 2MB range.

Andres

> On Thu, Oct 27, 2011 at 1:33 PM, Andres Lagar-Cavilla
> <andres@xxxxxxxxxxxxxxxx> wrote:
>>  xen/arch/x86/mm/hap/hap.c        |    2 +-
>>  xen/arch/x86/mm/hap/nested_hap.c |   21 ++-
>>  xen/arch/x86/mm/p2m-ept.c        |   26 +----
>>  xen/arch/x86/mm/p2m-pod.c        |   42 +++++--
>>  xen/arch/x86/mm/p2m-pt.c         |   20 +---
>>  xen/arch/x86/mm/p2m.c            |  185
>> ++++++++++++++++++++++++--------------
>>  xen/include/asm-ia64/mm.h        |    5 +
>>  xen/include/asm-x86/p2m.h        |   45 +++++++++-
>>  8 files changed, 217 insertions(+), 129 deletions(-)
>>
>>
>> This patch only modifies code internal to the p2m, adding convenience
>> macros, etc. It will yield a compiling code base but an incorrect
>> hypervisor (external callers of queries into the p2m will not unlock).
>> Next patch takes care of external callers, split done for the benefit
>> of conciseness.
>
> It's not obvious to me where in this patch to find a description of
> what the new locking regime is.  What does the _unlocked() mean?  When
> do I have to call that vs a different one, and when do I have to lock
> / unlock / whatever?
>
> I think that should ideally be both in the commit message (at least a
> summary), and also in a comment in a header somewhere.  Perhaps it is
> already in the patch somewhere, but a quick glance through didn't find
> it...
>
>>
>> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
>>
>> diff -r 8a98179666de -r 471d4f2754d6 xen/arch/x86/mm/hap/hap.c
>> --- a/xen/arch/x86/mm/hap/hap.c
>> +++ b/xen/arch/x86/mm/hap/hap.c
>> @@ -861,7 +861,7 @@ hap_write_p2m_entry(struct vcpu *v, unsi
>>     old_flags = l1e_get_flags(*p);
>>
>>     if ( nestedhvm_enabled(d) && (old_flags & _PAGE_PRESENT)
>> -         && !p2m_get_hostp2m(d)->defer_nested_flush ) {
>> +         && !atomic_read(&(p2m_get_hostp2m(d)->defer_nested_flush)) ) {
>>         /* We are replacing a valid entry so we need to flush nested
>> p2ms,
>>          * unless the only chan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel