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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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)? With this and with ...

> @@ -2222,7 +2228,14 @@ bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t 
> gpa,
>          domain_crash(hp2m->domain);
>      }
>  
> -    return 1;
> +    ret = true;
> +
> +put_hp2m:
> +    __put_gfn(hp2m, gfn_x(gfn));
> +put_ap2m:
> +    __put_gfn(*ap2m, gfn_x(gfn));

... label indentation once again corrected,
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan



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