[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/3] Introducing grant table V2 stucture
On Wed, 2011-11-09 at 08:14 +0000, annie.li@xxxxxxxxxx wrote: > diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c > index 4f44b34..0d481a9 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -53,7 +53,7 @@ > /* External tools reserve first few grant table entries. */ > #define NR_RESERVED_ENTRIES 8 > #define GNTTAB_LIST_END 0xffffffff > -#define GREFS_PER_GRANT_FRAME (PAGE_SIZE / sizeof(struct grant_entry)) > +#define GREFS_PER_GRANT_FRAME (PAGE_SIZE / sizeof(struct grant_entry_v1)) Does this need to become GREFS_V1_PER... or something? > > static grant_ref_t **gnttab_list; > static unsigned int nr_grant_frames; > @@ -64,7 +64,72 @@ static DEFINE_SPINLOCK(gnttab_list_lock); > unsigned long xen_hvm_resume_frames; > EXPORT_SYMBOL_GPL(xen_hvm_resume_frames); > > -static struct grant_entry *shared; > +static union { > + struct grant_entry_v1 *v1; > + void *ring_addr; > +} shared; > + > +/* > + * This function is null for grant table v1, adding it here in order to keep > + * consistent with *_v2 interface. > + */ > +static int gnttab_map_status_v1(unsigned int nr_gframes); > +/* > + * This function is null for grant table v1, adding it here in order to keep > + * consistent with *_v2 interface. > + */ > +static void gnttab_unmap_status_v1(void); If you reorder the declarations of gnttab_request_version and gnttab_(un)map_status_v1 below then you can avoid these forward declarations. Also I don't think the comment really adds much once you have the empty declaration underneath (like you do below). A simple "/* Nothing to do for v1 */" in the implementation would be sufficient. > + > +/*This is a structure of function pointers for grant table v1*/ and eventually v2, right? Actually I think the v1 is unnecessary. e.g.: /*This is a structure of function pointers for grant table*/ > +static struct { > + /* > + * Mapping a list of frames for storing grant entry status, this > + * mechanism can allow better synchronization using barriers. Input > + * parameter is frame number, returning GNTST_okay means success and > + * negative value means failure. > + */ > + int (*_gnttab_map_status)(unsigned int); I think you can drop the "_gnttab_" prefix from all of these, it'll be clear from the gnttab->foo what the namespace is. [...] > +} gnttab_interface; > + > +static int grant_table_version; > > static struct gnttab_free_callback *gnttab_free_callback_list; > > @@ -155,12 +229,10 @@ static void update_grant_entry(grant_ref_t ref, domid_t > domid, > * 3. Write memory barrier (WMB). > * 4. Write ent->flags, inc. valid type. > */ > - shared[ref].frame = frame; > - shared[ref].domid = domid; > - wmb(); > - shared[ref].flags = flags; > + gnttab_interface._update_grant_entry(ref, domid, frame, flags); > } > > + Please avoid unrelated whitespace changes. > /* > * Public grant-issuing interface functions > */ > @@ -187,31 +259,40 @@ int gnttab_grant_foreign_access(domid_t domid, unsigned > long frame, > } > EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access); > > -int gnttab_query_foreign_access(grant_ref_t ref) > +int gnttab_query_foreign_access_v1(grant_ref_t ref) This and all the other similar functions can now be made static. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |