Messages in this thread | | | From | Paul Durrant <> | Date | Wed, 7 Dec 2011 08:59:54 +0000 | Subject | RE: [PATCH 1/2] xen/granttable: Support sub-page grants |
| |
> -----Original Message----- > From: ANNIE LI [mailto:annie.li@oracle.com] > Sent: 07 December 2011 03:36 > To: Ian Campbell > Cc: xen-devel@lists.xensource.com; linux-kernel@vger.kernel.org; > konrad.wilk@oracle.com; jeremy@goop.org; kurt.hackel@oracle.com; > Paul Durrant > Subject: Re: [PATCH 1/2] xen/granttable: Support sub-page grants > > Thanks for your reviewing, Ian. > >> EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access); > >> > >> +int gnttab_grant_foreign_access_subpage_v2(domid_t domid, > unsigned long frame, > >> + int flags, unsigned page_off, > >> + unsigned length) > > Please drop the v2 suffixes on the public functions. > OK, the initial interface is without v2 suffixes. It was added in > order to reminder user the interfaces are only available for grant > table v2. > But I am fine to remove it, and following ops fn pointers are > better. > > Any reason not to route these via the ops table for consistency > with > > all the other ops? Then your availability check becomes a test for > > NULL fn pointer rather than a specific version. > Ok, it is good. > How about following implements? > > gnttab_v1_ops = { > ... > .access_subpage = NULL; > .access_ref_subpage = NULL; > .access_trans = NULL; > .access_ref_trans = NULL; > } > > gnttab_v2_ops = { > ... > .access_subpage = access_subpage_v2; > .access_ref_subpage = access_ref_subpage_v2; .access_trans = > access_trans_v2; .access_ref_trans = access_ref_trans_v2; } >
Do you need ops for the ref and non-ref functions? I would have thought you could just have the ref ones since the all the non-ref variants do is allocate and then call the ref variant.
Paul
> gnttab_request_version() > { > ..... > if(v2) > gnttab_interface = &gnttab_v2_ops; > else > gnttab_interface = &gnttab_v1_ops; ..... > } > > int gnttab_grant_foreign_access_subpage() > { > if(gnttab_interface->access_subpage != NULL) > return gnttab_interface->access_subpage; > return Esomething; > } > > Same operations for access_ref_subpage, access_trans and > access_ref_trans. > > bool gnttab_subpage_available() > { > return (gnttab_interface->access_subpage != NULL); } > > bool gnttab_subpage_available() > { > return (gnttab_interface->access_trans != NULL); } > > Thanks > Annie
| |