[Pv-ops][PATCH 2/4 v2] Netback: Introduce a new struct type page_ext.

[Pv-ops][PATCH 2/4 v2] Netback: Introduce a new struct type page_ext.

[Xu, Dongxiao]

[-- Attachment #1: Type: text/plain, Size: 214 bytes --]

Netback: Introduce a new struct type page_ext.

struct page_ext is used to store the group and idx information by
which a specified page could be identified.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

[-- Attachment #2: 0002-Netback-Introduce-a-new-struct-type-page_ext.patch --]
[-- Type: application/octet-stream, Size: 2949 bytes --]

From c3355a9294d9097259e0c5556ef9355bd2904b43 Mon Sep 17 00:00:00 2001
From: Dongxiao Xu <dongxiao.xu@intel.com>
Date: Fri, 30 Apr 2010 08:09:20 +0800
Subject: [PATCH] Netback: Introduce a new struct type page_ext.

struct page_ext is used to store the group and idx information by
which a specified page could be identified.

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 drivers/xen/netback/common.h  |    7 +++++++
 drivers/xen/netback/netback.c |   29 ++++++++++-------------------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index bcb7fa0..3b0838c 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -242,6 +242,11 @@ struct netbk_tx_pending_inuse {
 
 #define MAX_PENDING_REQS 256
 
+struct page_ext {
+	unsigned long group;
+	unsigned long idx;
+};
+
 struct xen_netbk {
 	struct tasklet_struct net_tx_tasklet;
 	struct tasklet_struct net_rx_tasklet;
@@ -254,6 +259,8 @@ struct xen_netbk {
 
 	struct page **mmap_pages;
 
+	struct page_ext page_extinfo[MAX_PENDING_REQS];
+
 	struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
 	struct netbk_tx_pending_inuse pending_inuse[MAX_PENDING_REQS];
 	struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index bd53972..8e942a1 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -79,22 +79,10 @@ static inline unsigned long idx_to_kaddr(unsigned int idx)
 }
 
 /* extra field used in struct page */
-static inline void netif_set_page_index(struct page *pg, unsigned int index)
+static inline void netif_set_page_index(struct page *pg,
+					struct page_ext *page_extinfo)
 {
-	*(unsigned long *)&pg->mapping = index + 1;
-}
-
-static inline int netif_page_index(struct page *pg)
-{
-	unsigned long idx = (unsigned long)pg->mapping - 1;
-
-	if (!PageForeign(pg))
-		return -1;
-
-	if ((idx >= MAX_PENDING_REQS) || (netbk->mmap_pages[idx] != pg))
-		return -1;
-
-	return idx;
+	pg->mapping = (struct address_space *)page_extinfo;;
 }
 
 /*
@@ -1411,9 +1399,10 @@ static void netif_idx_release(u16 pending_idx)
 
 static void netif_page_release(struct page *page, unsigned int order)
 {
-	int idx = netif_page_index(page);
-	BUG_ON(order);
-	BUG_ON(idx < 0);
+	int idx = ((struct page_ext *)(page->mapping))->idx;
+	BUG_ON(order ||
+	       idx < 0 || idx >= MAX_PENDING_REQS ||
+	       netbk->mmap_pages[idx] != page);
 	netif_idx_release(idx);
 }
 
@@ -1565,7 +1554,9 @@ static int __init netback_init(void)
 	for (i = 0; i < MAX_PENDING_REQS; i++) {
 		page = netbk->mmap_pages[i];
 		SetPageForeign(page, netif_page_release);
-		netif_set_page_index(page, i);
+		netbk->page_extinfo[i].group = 0;
+		netbk->page_extinfo[i].idx = i;
+		netif_set_page_index(page, &netbk->page_extinfo[i]);
 		INIT_LIST_HEAD(&netbk->pending_inuse[i].list);
 	}
 
-- 
1.6.0


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [Pv-ops][PATCH 2/4 v2] Netback: Introduce a new struct type page_ext.

[Konrad Rzeszutek Wilk]

On Thu, Apr 29, 2010 at 10:28:36PM +0800, Xu, Dongxiao wrote:
> Netback: Introduce a new struct type page_ext.
> 
> struct page_ext is used to store the group and idx information by
> which a specified page could be identified.
> 
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

.. snip.. 


-static inline int netif_page_index(struct page *pg)
-{
-	unsigned long idx = (unsigned long)pg->mapping - 1;
-
-	if (!PageForeign(pg))
-		return -1;

You are taking the check to see if the page is foreign. Is that OK?
-
-	if ((idx >= MAX_PENDING_REQS) || (netbk->mmap_pages[idx] != pg))
-		return -1;
-
-	return idx;
+	pg->mapping = (struct address_space *)page_extinfo;;
 }
 
 /*
@@ -1411,9 +1399,10 @@ static void netif_idx_release(u16 pending_idx)
 
 static void netif_page_release(struct page *page, unsigned int order)
 {
-	int idx = netif_page_index(page);

This did this nice little check to see if the page had the PageForeign
bit set. Is it OK to remove that check?

-	BUG_ON(order);
-	BUG_ON(idx < 0);
+	int idx = ((struct page_ext *)(page->mapping))->idx;
+	BUG_ON(order ||
+	       idx < 0 || idx >= MAX_PENDING_REQS ||
+	       netbk->mmap_pages[idx] != page);
 	netif_idx_release(idx);
 }
 
@@ -1565,7 +1554,9 @@ static int __init netback_init(void)
 	for (i = 0; i < MAX_PENDING_REQS; i++) {
 		page = netbk->mmap_pages[i];
 		SetPageForeign(page, netif_page_release);
-		netif_set_page_index(page, i);
+		netbk->page_extinfo[i].group = 0;
+		netbk->page_extinfo[i].idx = i;
+		netif_set_page_index(page, &netbk->page_extinfo[i]);
 		INIT_LIST_HEAD(&netbk->pending_inuse[i].list);
 	}
 
-- 
1.6.0

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [Pv-ops][PATCH 2/4 v2] Netback: Introduce a new struct type page_ext.

[Xu, Dongxiao]

I will add this check, thanks for your reminder.

Thanks,
Dongxiao

Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 29, 2010 at 10:28:36PM +0800, Xu, Dongxiao wrote:
>> Netback: Introduce a new struct type page_ext.
>> 
>> struct page_ext is used to store the group and idx information by
>> which a specified page could be identified.
>> 
>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> 
> .. snip..
> 
> 
> -static inline int netif_page_index(struct page *pg)
> -{
> -	unsigned long idx = (unsigned long)pg->mapping - 1;
> -
> -	if (!PageForeign(pg))
> -		return -1;
> 
> You are taking the check to see if the page is foreign. Is that OK?
> -
> -	if ((idx >= MAX_PENDING_REQS) || (netbk->mmap_pages[idx] != pg))
> -		return -1;
> -
> -	return idx;
> +	pg->mapping = (struct address_space *)page_extinfo;;
>  }
> 
>  /*
> @@ -1411,9 +1399,10 @@ static void netif_idx_release(u16 pending_idx)
> 
>  static void netif_page_release(struct page *page, unsigned int order)
>  {
> -	int idx = netif_page_index(page);
> 
> This did this nice little check to see if the page had the PageForeign
> bit set. Is it OK to remove that check?
> 
> -	BUG_ON(order);
> -	BUG_ON(idx < 0);
> +	int idx = ((struct page_ext *)(page->mapping))->idx;
> +	BUG_ON(order ||
> +	       idx < 0 || idx >= MAX_PENDING_REQS ||
> +	       netbk->mmap_pages[idx] != page);
>  	netif_idx_release(idx);
>  }
> 
> @@ -1565,7 +1554,9 @@ static int __init netback_init(void)
>  	for (i = 0; i < MAX_PENDING_REQS; i++) {
>  		page = netbk->mmap_pages[i];
>  		SetPageForeign(page, netif_page_release);
> -		netif_set_page_index(page, i);
> +		netbk->page_extinfo[i].group = 0;
> +		netbk->page_extinfo[i].idx = i;
> +		netif_set_page_index(page, &netbk->page_extinfo[i]);
>  		INIT_LIST_HEAD(&netbk->pending_inuse[i].list);
>  	}
> 
> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel