[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] gnttab: Introduce rwlock to protect updates to grant table state
On 02/12/14 10:06, Christoph Egger wrote: > Rename lock to maptrack_lock and use it to protect maptrack state. > > The new rwlock is used to prevent readers from accessing > inconsistent grant table state such as current > version, partially initialized active table pages, > etc. I would suggest phrasing this slightly differently, as it isn't a simple rename. What you are doing is taking the existing grant table lock, splitting it in two to separate the grant locking from the maptrack locking, and changing the resulting grant lock to be a rwlock. With this noted, it would certainly be easier to review if this patch was split in two; one patch to split the spinlocks and one patch to change the grant lock from a spinlock to a rwlock. > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index 8fba923..018b5b6 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -2501,8 +2510,19 @@ > gnttab_swap_grant_ref(XEN_GUEST_HANDLE_PARAM(gnttab_swap_grant_ref_t) uop, > return i; > if ( unlikely(__copy_from_guest(&op, uop, 1)) ) > return -EFAULT; > - op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); > - if ( unlikely(__copy_field_to_guest(uop, &op, status)) ) > + if ( unlikely(op.ref_a == op.ref_b) ) > + { > + /* noop, so avoid acquiring the same active entry > + * twice in __gnttab_swap_grant_ref(), which would > + * case a deadlock. > + */ > + op.status = GNTST_okay; > + } > + else > + { > + op.status = __gnttab_swap_grant_ref(op.ref_a, op.ref_b); > + } > + if ( unlikely(__copy_to_guest_offset(uop, i, &op, 1)) ) I believe this comment only applies to the changes in active locking introduced in the subsequent patch? Either way, I think the a == b check should be in __gnttab_swap_grant_ref() make any caller safe, not just the this one. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |