[PATCH] minios: free netfront resources on error/shutdown

[PATCH] minios: free netfront resources on error/shutdown

[Samuel Thibault]

minios: free netfront resources on error/shutdown

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r b0d7780794eb extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/fbfront.c	Mon May 12 11:33:51 2008 +0200
@@ -210,7 +210,7 @@
 
     unbind_evtchn(dev->evtchn);
 
-    free_pages(dev->page,0);
+    free_page(dev->page);
     free(nodename);
     free(dev->backend);
     free(dev);
@@ -477,7 +477,7 @@
 
     unbind_evtchn(dev->evtchn);
 
-    free_pages(dev->page,0);
+    free_page(dev->page);
     free(nodename);
     free(dev->backend);
     free(dev);
diff -r b0d7780794eb extras/mini-os/include/mm.h
--- a/extras/mini-os/include/mm.h	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/include/mm.h	Mon May 12 11:33:51 2008 +0200
@@ -48,6 +48,7 @@
 unsigned long alloc_pages(int order);
 #define alloc_page()    alloc_pages(0)
 void free_pages(void *pointer, int order);
+#define free_page(p)    free_pages(p, 0)
 
 static __inline__ int get_order(unsigned long size)
 {
diff -r b0d7780794eb extras/mini-os/lib/xmalloc.c
--- a/extras/mini-os/lib/xmalloc.c	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/lib/xmalloc.c	Mon May 12 11:33:51 2008 +0200
@@ -264,7 +264,7 @@
             printk("Bug\n");
             *(int*)0=0;
         }
-        free_pages(hdr, 0);
+        free_page(hdr);
     }
     else
     {
diff -r b0d7780794eb extras/mini-os/netfront.c
--- a/extras/mini-os/netfront.c	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/netfront.c	Mon May 12 11:33:51 2008 +0200
@@ -52,6 +52,7 @@
 
     char *nodename;
     char *backend;
+    char *mac;
 
     xenbus_event_queue events;
 
@@ -263,6 +264,39 @@
 }
 #endif
 
+static void free_netfront(struct netfront_dev *dev)
+{
+    int i;
+
+    for(i=0;i<NET_TX_RING_SIZE;i++)
+	down(&dev->tx_sem);
+
+    mask_evtchn(dev->evtchn);
+
+    free(dev->mac);
+    free(dev->backend);
+
+    gnttab_end_access(dev->rx_ring_ref);
+    gnttab_end_access(dev->tx_ring_ref);
+
+    free_page(dev->rx.sring);
+    free_page(dev->tx.sring);
+
+    unbind_evtchn(dev->evtchn);
+
+    for(i=0;i<NET_RX_RING_SIZE;i++) {
+	gnttab_end_access(dev->rx_buffers[i].gref);
+	free_page(dev->rx_buffers[i].page);
+    }
+
+    for(i=0;i<NET_TX_RING_SIZE;i++)
+	if (dev->tx_buffers[i].page)
+	    free_page(dev->tx_buffers[i].page);
+
+    free(dev->nodename);
+    free(dev);
+}
+
 struct netfront_dev *init_netfront(char *nodename, void (*thenetif_rx)(unsigned char* data, int len), unsigned char rawmac[6], char **ip)
 {
     xenbus_transaction_t xbt;
@@ -272,7 +306,6 @@
     struct netif_rx_sring *rxs;
     int retry=0;
     int i;
-    char* mac;
     char* msg;
 
     struct netfront_dev *dev;
@@ -288,6 +321,7 @@
     printk("************************ NETFRONT for %s **********\n\n\n", nodename);
 
     dev = malloc(sizeof(*dev));
+    memset(dev, 0, sizeof(*dev));
     dev->nodename = strdup(nodename);
 
     printk("net TX ring size %d\n", NET_TX_RING_SIZE);
@@ -314,7 +348,7 @@
 #endif
         evtchn_alloc_unbound(dev->dom, netfront_handler, dev, &dev->evtchn);
 
-    txs = (struct netif_tx_sring*) alloc_page();
+    txs = (struct netif_tx_sring *) alloc_page();
     rxs = (struct netif_rx_sring *) alloc_page();
     memset(txs,0,PAGE_SIZE);
     memset(rxs,0,PAGE_SIZE);
@@ -328,11 +362,12 @@
     dev->tx_ring_ref = gnttab_grant_access(dev->dom,virt_to_mfn(txs),0);
     dev->rx_ring_ref = gnttab_grant_access(dev->dom,virt_to_mfn(rxs),0);
 
+    init_rx_buffers(dev);
+
     dev->netif_rx = thenetif_rx;
 
     dev->events = NULL;
 
-    // FIXME: proper frees on failures
 again:
     err = xenbus_transaction_start(&xbt);
     if (err) {
@@ -379,25 +414,22 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
     snprintf(path, sizeof(path), "%s/backend", nodename);
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     snprintf(path, sizeof(path), "%s/mac", nodename);
-    msg = xenbus_read(XBT_NIL, path, &mac);
+    msg = xenbus_read(XBT_NIL, path, &dev->mac);
 
-    if ((dev->backend == NULL) || (mac == NULL)) {
-        struct evtchn_close op = { dev->evtchn };
+    if ((dev->backend == NULL) || (dev->mac == NULL)) {
         printk("%s: backend/mac failed\n", __func__);
-        unbind_evtchn(dev->evtchn);
-        HYPERVISOR_event_channel_op(EVTCHNOP_close, &op);
-        return NULL;
+        goto error;
     }
 
     printk("backend at %s\n",dev->backend);
-    printk("mac is %s\n",mac);
+    printk("mac is %s\n",dev->mac);
 
     {
         char path[strlen(dev->backend) + 1 + 5 + 1];
@@ -415,13 +447,12 @@
 
     printk("**************************\n");
 
-    init_rx_buffers(dev);
     unmask_evtchn(dev->evtchn);
 
         /* Special conversion specifier 'hh' needed for __ia64__. Without
            this mini-os panics with 'Unaligned reference'. */
     if (rawmac)
-	sscanf(mac,"%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+	sscanf(dev->mac,"%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
             &rawmac[0],
             &rawmac[1],
             &rawmac[2],
@@ -430,6 +461,9 @@
             &rawmac[5]);
 
     return dev;
+error:
+    free_netfront(dev);
+    return NULL;
 }
 
 #ifdef HAVE_LIBC
@@ -467,11 +501,7 @@
 
     xenbus_unwatch_path(XBT_NIL, path);
 
-    unbind_evtchn(dev->evtchn);
-
-    free(nodename);
-    free(dev->backend);
-    free(dev);
+    free_netfront(dev);
 }

minios: free blkfront resources on error/shutdown

[Samuel Thibault]

minios: free blkfront resources on error/shutdown

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r b0d7780794eb extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/blkfront.c	Mon May 12 11:58:01 2008 +0200
@@ -68,6 +68,21 @@
     wake_up(&blkfront_queue);
 }
 
+static void free_blkfront(struct blkfront_dev *dev)
+{
+    mask_evtchn(dev->evtchn);
+
+    free(dev->backend);
+
+    gnttab_end_access(dev->ring_ref);
+    free_page(dev->ring.sring);
+
+    unbind_evtchn(dev->evtchn);
+
+    free(dev->nodename);
+    free(dev);
+}
+
 struct blkfront_dev *init_blkfront(char *nodename, struct blkfront_info *info)
 {
     xenbus_transaction_t xbt;
@@ -88,6 +103,7 @@
     printk("******************* BLKFRONT for %s **********\n\n\n", nodename);
 
     dev = malloc(sizeof(*dev));
+    memset(dev, 0, sizeof(*dev));
     dev->nodename = strdup(nodename);
 
     snprintf(path, sizeof(path), "%s/backend-id", nodename);
@@ -139,7 +155,7 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
@@ -147,7 +163,7 @@
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     if (msg) {
         printk("Error %s when reading the backend path %s\n", msg, path);
-        return NULL;
+        goto error;
     }
 
     printk("backend at %s\n", dev->backend);
@@ -160,7 +176,7 @@
         msg = xenbus_read(XBT_NIL, path, &c);
         if (msg) {
             printk("Error %s when reading the mode\n", msg);
-            return NULL;
+            goto error;
         }
         if (*c == 'w')
             dev->info.mode = O_RDWR;
@@ -198,6 +214,10 @@
     printk("**************************\n");
 
     return dev;
+
+error:
+    free_blkfront(dev);
+    return NULL;
 }
 
 void shutdown_blkfront(struct blkfront_dev *dev)
@@ -220,11 +240,7 @@
 
     xenbus_unwatch_path(XBT_NIL, path);
 
-    unbind_evtchn(dev->evtchn);
-
-    free(nodename);
-    free(dev->backend);
-    free(dev);
+    free_blkfront(dev);
 }
 
 static void blkfront_wait_slot(struct blkfront_dev *dev)

[PATCH] minios: free fbfront resources on error/shutdown

[Samuel Thibault]

minios: free fbfront resources on error/shutdown

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r b0d7780794eb extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c	Thu May 08 13:40:40 2008 +0100
+++ b/extras/mini-os/fbfront.c	Mon May 12 12:10:30 2008 +0200
@@ -47,6 +47,20 @@
     files[fd].read = 1;
 #endif
     wake_up(&kbdfront_queue);
+}
+
+static void free_kbdfront(struct kbdfront_dev *dev)
+{
+    mask_evtchn(dev->evtchn);
+
+    free(dev->backend);
+
+    free_page(dev->page);
+
+    unbind_evtchn(dev->evtchn);
+
+    free(dev->nodename);
+    free(dev);
 }
 
 struct kbdfront_dev *init_kbdfront(char *nodename, int abs_pointer)
@@ -122,7 +136,7 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
@@ -130,7 +144,7 @@
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     if (msg) {
         printk("Error %s when reading the backend path %s\n", msg, path);
-        return NULL;
+	goto error;
     }
 
     printk("backend at %s\n", dev->backend);
@@ -153,6 +167,9 @@
     printk("************************** KBDFRONT\n");
 
     return dev;
+error:
+    free_kbdfront(dev);
+    return NULL;
 }
 
 int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n)
@@ -208,12 +225,7 @@
 
     xenbus_unwatch_path(XBT_NIL, path);
 
-    unbind_evtchn(dev->evtchn);
-
-    free_pages(dev->page,0);
-    free(nodename);
-    free(dev->backend);
-    free(dev);
+    free_kbdfront(dev);
 }
 
 #ifdef HAVE_LIBC
@@ -260,6 +272,20 @@
 void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
 {
     wake_up(&fbfront_queue);
+}
+
+static void free_fbfront(struct fbfront_dev *dev)
+{
+    mask_evtchn(dev->evtchn);
+
+    free(dev->backend);
+
+    free_page(dev->page);
+
+    unbind_evtchn(dev->evtchn);
+
+    free(dev->nodename);
+    free(dev);
 }
 
 struct fbfront_dev *init_fbfront(char *nodename, unsigned long *mfns, int width, int height, int depth, int stride, int n)
@@ -357,7 +383,7 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
@@ -365,7 +391,7 @@
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     if (msg) {
         printk("Error %s when reading the backend path %s\n", msg, path);
-        return NULL;
+        goto error;
     }
 
     printk("backend at %s\n", dev->backend);
@@ -391,6 +417,10 @@
     printk("************************** FBFRONT\n");
 
     return dev;
+
+error:
+    free_fbfront(dev);
+    return NULL;
 }
 
 static void fbfront_out_event(struct fbfront_dev *dev, union xenfb_out_event *event)
@@ -477,7 +507,7 @@
 
     unbind_evtchn(dev->evtchn);
 
-    free_pages(dev->page,0);
+    free_page(dev->page);
     free(nodename);
     free(dev->backend);
     free(dev);

Re: [PATCH] minios: free fbfront resources on error/shutdown

[Keir Fraser]

Doesn't apply to xen-unstable tip c/s 17616.

 -- Keir

On 12/5/08 11:11, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:

> minios: free fbfront resources on error/shutdown
> 
> Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
> 
> diff -r b0d7780794eb extras/mini-os/fbfront.c
> --- a/extras/mini-os/fbfront.c Thu May 08 13:40:40 2008 +0100
> +++ b/extras/mini-os/fbfront.c Mon May 12 12:10:30 2008 +0200
> @@ -47,6 +47,20 @@
>      files[fd].read = 1;
>  #endif
>      wake_up(&kbdfront_queue);
> +}
> +
> +static void free_kbdfront(struct kbdfront_dev *dev)
> +{
> +    mask_evtchn(dev->evtchn);
> +
> +    free(dev->backend);
> +
> +    free_page(dev->page);
> +
> +    unbind_evtchn(dev->evtchn);
> +
> +    free(dev->nodename);
> +    free(dev);
>  }
>  
>  struct kbdfront_dev *init_kbdfront(char *nodename, int abs_pointer)
> @@ -122,7 +136,7 @@
>  
>  abort_transaction:
>      xenbus_transaction_end(xbt, 1, &retry);
> -    return NULL;
> +    goto error;
>  
>  done:
>  
> @@ -130,7 +144,7 @@
>      msg = xenbus_read(XBT_NIL, path, &dev->backend);
>      if (msg) {
>          printk("Error %s when reading the backend path %s\n", msg, path);
> -        return NULL;
> + goto error;
>      }
>  
>      printk("backend at %s\n", dev->backend);
> @@ -153,6 +167,9 @@
>      printk("************************** KBDFRONT\n");
>  
>      return dev;
> +error:
> +    free_kbdfront(dev);
> +    return NULL;
>  }
>  
>  int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf,
> int n)
> @@ -208,12 +225,7 @@
>  
>      xenbus_unwatch_path(XBT_NIL, path);
>  
> -    unbind_evtchn(dev->evtchn);
> -
> -    free_pages(dev->page,0);
> -    free(nodename);
> -    free(dev->backend);
> -    free(dev);
> +    free_kbdfront(dev);
>  }
>  
>  #ifdef HAVE_LIBC
> @@ -260,6 +272,20 @@
>  void fbfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
>  {
>      wake_up(&fbfront_queue);
> +}
> +
> +static void free_fbfront(struct fbfront_dev *dev)
> +{
> +    mask_evtchn(dev->evtchn);
> +
> +    free(dev->backend);
> +
> +    free_page(dev->page);
> +
> +    unbind_evtchn(dev->evtchn);
> +
> +    free(dev->nodename);
> +    free(dev);
>  }
>  
>  struct fbfront_dev *init_fbfront(char *nodename, unsigned long *mfns, int
> width, int height, int depth, int stride, int n)
> @@ -357,7 +383,7 @@
>  
>  abort_transaction:
>      xenbus_transaction_end(xbt, 1, &retry);
> -    return NULL;
> +    goto error;
>  
>  done:
>  
> @@ -365,7 +391,7 @@
>      msg = xenbus_read(XBT_NIL, path, &dev->backend);
>      if (msg) {
>          printk("Error %s when reading the backend path %s\n", msg, path);
> -        return NULL;
> +        goto error;
>      }
>  
>      printk("backend at %s\n", dev->backend);
> @@ -391,6 +417,10 @@
>      printk("************************** FBFRONT\n");
>  
>      return dev;
> +
> +error:
> +    free_fbfront(dev);
> +    return NULL;
>  }
>  
>  static void fbfront_out_event(struct fbfront_dev *dev, union xenfb_out_event
> *event)
> @@ -477,7 +507,7 @@
>  
>      unbind_evtchn(dev->evtchn);
>  
> -    free_pages(dev->page,0);
> +    free_page(dev->page);
>      free(nodename);
>      free(dev->backend);
>      free(dev);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] minios: free fbfront resources on error/shutdown

[Samuel Thibault]

Keir Fraser, le Mon 12 May 2008 11:17:47 +0100, a écrit :
> Doesn't apply to xen-unstable tip c/s 17616.

Here is a fixed patch


minios: free fbfront resources on error/shutdown

Signed-off-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

diff -r 2d8d5b7ada6f extras/mini-os/fbfront.c
--- a/extras/mini-os/fbfront.c	Mon May 12 14:14:45 2008 +0200
+++ b/extras/mini-os/fbfront.c	Mon May 12 14:15:31 2008 +0200
@@ -47,6 +47,20 @@
     files[fd].read = 1;
 #endif
     wake_up(&kbdfront_queue);
+}
+
+static void free_kbdfront(struct kbdfront_dev *dev)
+{
+    mask_evtchn(dev->evtchn);
+
+    free(dev->backend);
+
+    free_page(dev->page);
+
+    unbind_evtchn(dev->evtchn);
+
+    free(dev->nodename);
+    free(dev);
 }
 
 struct kbdfront_dev *init_kbdfront(char *nodename, int abs_pointer)
@@ -122,7 +136,7 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
@@ -130,7 +144,7 @@
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     if (msg) {
         printk("Error %s when reading the backend path %s\n", msg, path);
-        return NULL;
+        goto error;
     }
 
     printk("backend at %s\n", dev->backend);
@@ -153,6 +167,9 @@
     printk("************************** KBDFRONT\n");
 
     return dev;
+error:
+    free_kbdfront(dev);
+    return NULL;
 }
 
 int kbdfront_receive(struct kbdfront_dev *dev, union xenkbd_in_event *buf, int n)
@@ -208,12 +225,7 @@
 
     xenbus_unwatch_path(XBT_NIL, path);
 
-    unbind_evtchn(dev->evtchn);
-
-    free_page(dev->page);
-    free(nodename);
-    free(dev->backend);
-    free(dev);
+    free_kbdfront(dev);
 }
 
 #ifdef HAVE_LIBC
@@ -270,6 +282,20 @@
     files[fd].read = 1;
 #endif
     wake_up(&fbfront_queue);
+}
+
+static void free_fbfront(struct fbfront_dev *dev)
+{
+    mask_evtchn(dev->evtchn);
+
+    free(dev->backend);
+
+    free_page(dev->page);
+
+    unbind_evtchn(dev->evtchn);
+
+    free(dev->nodename);
+    free(dev);
 }
 
 int fbfront_receive(struct fbfront_dev *dev, union xenfb_in_event *buf, int n)
@@ -401,7 +427,7 @@
 
 abort_transaction:
     xenbus_transaction_end(xbt, 1, &retry);
-    return NULL;
+    goto error;
 
 done:
 
@@ -409,7 +435,7 @@
     msg = xenbus_read(XBT_NIL, path, &dev->backend);
     if (msg) {
         printk("Error %s when reading the backend path %s\n", msg, path);
-        return NULL;
+        goto error;
     }
 
     printk("backend at %s\n", dev->backend);
@@ -435,6 +461,10 @@
     printk("************************** FBFRONT\n");
 
     return dev;
+
+error:
+    free_fbfront(dev);
+    return NULL;
 }
 
 static void fbfront_out_event(struct fbfront_dev *dev, union xenfb_out_event *event)
@@ -521,10 +551,7 @@
 
     unbind_evtchn(dev->evtchn);
 
-    free_page(dev->page);
-    free(nodename);
-    free(dev->backend);
-    free(dev);
+    free_fbfront(dev);
 }
 
 #ifdef HAVE_LIBC