On 22/11/2018 15:01, Jan Beulich wrote: >>>> On 21.11.18 at 14:21, <andrew.coop...@citrix.com> 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel