[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/3] Introducing grant table V2 stucture
On 2011-11-9 21:28, Ian Campbell wrote:
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?
This is common definition for both V1 and V2, and is used by both v1
and v2 implementation. If it is changed to GREFS_V1_PER... or something
with v1 tag in this patch, it is necessary to change it back in
following "grant table v2 implementation" patch again. So I just keep
it unchanged here.
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.
Yes, you are right, thanks.
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.
I agree.
+
+/*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*/
Thanks, will change it.
+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.
Ok, no problem.
[...]
+} 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.
Oh, it was ignored. Thanks, i will delete it.
/*
* 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.
Ok, got it, thanks.
Thanks
Annie
Ian.
|
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|