[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 02/11] xen/gnttab: Rework resource acquisition
On 24/09/2020 10:51, Paul Durrant wrote: >> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c >> index a5d3ed8bda..912f07be47 100644 >> --- a/xen/common/grant_table.c >> +++ b/xen/common/grant_table.c >> @@ -4013,6 +4013,81 @@ static int gnttab_get_shared_frame_mfn(struct domain >> *d, >> return 0; >> } >> >> +int gnttab_acquire_resource( >> + struct domain *d, unsigned int id, unsigned long frame, >> + unsigned int nr_frames, xen_pfn_t mfn_list[]) >> +{ >> + struct grant_table *gt = d->grant_table; >> + unsigned int i = nr_frames, tot_frames; >> + mfn_t tmp; >> + void **vaddrs = NULL; >> + int rc; >> + >> + /* Input sanity. */ > Nit: inconsistency with full stops on single line comments. The whole point of relaxing the style was because feedback over minutia such as this was deemed detrimental to the community. If I ever see feedback like this, I will commit commit the patch there and then. This is the only way upstream Xen is going to turn into a less toxic environment for contributors. >> + rc = -EINVAL; >> + break; >> + } >> + rc = gnttab_get_status_frame_mfn(d, tot_frames - 1, &tmp); >> + break; >> + } >> + > I think you could drop the write lock here... > >> + /* Any errors from growing the table? */ >> + if ( rc ) >> + goto out; >> + > ...and acquire it read here, since we know the table cannot shrink. You'd > need to re-check the gt_version for safety though. And you've correctly identified why I didn't. If we had a relax-write-to-read lock operation, that would also be fine, but we don't. Fundamentally, this is an operation made once during VM construction, to map one single frame. It is not a hotpath in need of microptimising its locking pattern, and absolutely not something worth introducing a safety hazard for. ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |