Re: [Xen-devel] [PATCH 04/14] x86/p2m: Fix locking in p2m_altp2m_lazy_copy()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/11/2018 15:01, Jan Beulich wrote:
>>>> On 21.11.18 at 14:21, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -2184,24 +2184,29 @@ bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t 
>> gpa,
>>      unsigned long mask;
>>      mfn_t mfn;
>>      int rv;
>> +    bool ret;
>>  
>>      *ap2m = p2m_get_altp2m(v);
>>  
>>      mfn = get_gfn_type_access(*ap2m, gfn_x(gfn), &p2mt, &p2ma,
>>                                0, &page_order);
>> -    __put_gfn(*ap2m, gfn_x(gfn));
>>  
>> +    /* Entry already present in ap2m?  Caller should handle the fault. */
>>      if ( !mfn_eq(mfn, INVALID_MFN) )
>> -        return 0;
>> +    {
>> +        ret = false;
>> +        goto put_ap2m;
>> +    }
>>  
>>      mfn = get_gfn_type_access(hp2m, gfn_x(gfn), &p2mt, &p2ma,
>>                                P2M_ALLOC, &page_order);
>> -    __put_gfn(hp2m, gfn_x(gfn));
>>  
>> +    /* Entry not present in hp2m?  Caller should handle the fault. */
>>      if ( mfn_eq(mfn, INVALID_MFN) )
>> -        return 0;
>> -
>> -    p2m_lock(*ap2m);
>> +    {
>> +        ret = false;
>> +        goto put_hp2m;
> Wouldn't this better be named "put_gfn" (or "drop_gfn" to avoid the
> name collision with the function)?

No - I don't think so.  We have two separate refs to drop, one from the
hp2m and one from the ap2m.

This is why the labels are named symmetrically.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel