On Tue, 2012-01-31 at 11:09 +0000, Wei Liu (Intern) wrote:

On Tue, 2012-01-31 at 09:01 +0000, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 30.01.12 at 18:10, Wei Liu [off-list ref] wrote:

On Mon, 2012-01-30 at 16:35 +0000, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 30.01.12 at 15:45, Wei Liu [off-list ref] wrote:

-int xenvif_map_frontend_rings(struct xenvif *vif,
-			      grant_ref_t tx_ring_ref,
-			      grant_ref_t rx_ring_ref)
+int xenvif_map_frontend_rings(struct xen_comms *comms,
+			      int domid,
+			      unsigned long ring_ref[],
+			      unsigned int  ring_ref_count)
 {
-	void *addr;
-	struct xen_netif_tx_sring *txs;
-	struct xen_netif_rx_sring *rxs;
-
-	int err = -ENOMEM;
+	struct gnttab_map_grant_ref op[NETBK_MAX_RING_PAGES];
+	unsigned int i;
+	int err = 0;
 
-	err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif),
-				     tx_ring_ref, &addr);

Any reason why you don't just extend this function (in a prerequisite
patch) rather than open coding a common utility function (twice) here,
so that other backends (blkback!) can benefit later as well.

Jan

I'm mainly focusing on netback stuffs, so the code is slightly coupled
with netback -- NETBK_MAX_RING_PAGES.

To extend xenbus_map_ring_valloc and make more generic, it requires
setting a global maximum page number limits on rings, I think it will
require further investigation and code refactor -- which I have no time
to attend to at the moment. :-/

Why? You can simply pass in the number of pages, there's no need
for a global maximum.

I mean the gnttab_map_gran_ref array, it is statically allocated at the
moment. Of course we can make it dynamically allocated, but why bother
taking the risk of allocation failure.

You can do
	struct gnttab_map_grant_ref op[nr_pages];
and the compiler will do the right thing with the on-stack data
structure. In the kernel you'd need to be a bit careful about the size
of pages first but that should be all.

Ian.