Skip to contentSkip to conversation

minios: blkfront_aio_poll may reenter

PATCH In review Samuel Thibault 1 participant 3 messages Mar 26, 2008 – Apr 5, 2008

lore
Share & tools
lore

Samuel Thibault

author
#0 lore raw

PATCH 1/1 minios: blkfront_aio_poll may reenter

minios: blkfront_aio_poll may reenter
if the callback calls blkfront_sync for instance.
In such a case, we would see responses and hence release grants several times. We huts need to be more synchronous and stop when we detect that we have re-entered.
This fixes HVM restore with stubdomains.

Signed-off-by Samuel Thibault <[email protected]>
extras/mini-os/blkfront.c +13 -10
diff -r 91c4e704169d extras/mini-os/blkfront.c
@@ -319,6 +319,7 @@ int blkfront_aio_poll(struct blkfront_de
 {
     RING_IDX rp, cons;
     struct blkif_response *rsp;
+    int more;
 
 moretodo:
 #ifdef HAVE_LIBC

@@ -334,6 +335,7 @@ moretodo:
     while ((cons != rp))
     {
 	rsp = RING_GET_RESPONSE(&dev->ring, cons);
+	nr_consumed++;
 
         if (rsp->status != BLKIF_RSP_OKAY)
             printk("block error %d for op %d\n", rsp->status, rsp->operation);

@@ -343,29 +345,30 @@ moretodo:
         case BLKIF_OP_WRITE:
         {
             struct blkfront_aiocb *aiocbp = (void*) (uintptr_t) rsp->id;
+            int status = rsp->status;
             int j;
 
             for (j = 0; j < aiocbp->n; j++)
                 gnttab_end_access(aiocbp->gref[j]);
 
+            dev->ring.rsp_cons = ++cons;
             /* Nota: callback frees aiocbp itself */
-            aiocbp->aio_cb(aiocbp, rsp->status ? -EIO : 0);
+            aiocbp->aio_cb(aiocbp, status ? -EIO : 0);
+            if (dev->ring.rsp_cons != cons)
+                /* We reentered, we must not continue here */
+                goto out;
             break;
         }
+        default:
+            printk("unrecognized block operation %d response\n", rsp->operation);
         case BLKIF_OP_WRITE_BARRIER:
         case BLKIF_OP_FLUSH_DISKCACHE:
-            break;
-        default:
-            printk("unrecognized block operation %d response\n", rsp->operation);
+            dev->ring.rsp_cons = ++cons;
             break;
         }
+    }
 
-	nr_consumed++;
-	++cons;
-    }
-    dev->ring.rsp_cons = cons;
-
-    int more;
+out:
     RING_FINAL_CHECK_FOR_RESPONSES(&dev->ring, more);
     if (more) goto moretodo;
extras/mini-os/minios.mk +4 -4
diff -r 91c4e704169d extras/mini-os/minios.mk
@@ -16,10 +16,10 @@ DEF_LDFLAGS =
 
 ifeq ($(debug),y)
 DEF_CFLAGS += -g
-#DEF_CFLAGS += MM_DEBUG
-#DEF_CFLAGS += FS_DEBUG
-#DEF_CFLAGS += LIBC_DEBUG
-#DEF_CFLAGS += GNT_DEBUG
+#DEF_CFLAGS += -DMM_DEBUG
+#DEF_CFLAGS += -DFS_DEBUG
+#DEF_CFLAGS += -DLIBC_DEBUG
+DEF_CFLAGS += -DGNT_DEBUG
 else
 DEF_CFLAGS += -O3
 endif
author
#1 lore raw

Hello,

That patch didn't get apply, is there any problem with it?

Samuel

minios: blkfront_aio_poll may reenter
if the callback calls blkfront_sync for instance.
In such a case, we would see responses and hence release grants several times. We thus need to be more synchronous and stop when we detect that we have re-entered.
This fixes HVM restore with stubdomains.

Signed-off-by Samuel Thibault <[email protected]>
extras/mini-os/blkfront.c +13 -10
diff -r 91c4e704169d extras/mini-os/blkfront.c
@@ -319,6 +319,7 @@ int blkfront_aio_poll(struct blkfront_de
 {
     RING_IDX rp, cons;
     struct blkif_response *rsp;
+    int more;
 
 moretodo:
 #ifdef HAVE_LIBC

@@ -334,6 +335,7 @@ moretodo:
     while ((cons != rp))
     {
 	rsp = RING_GET_RESPONSE(&dev->ring, cons);
+	nr_consumed++;
 
         if (rsp->status != BLKIF_RSP_OKAY)
             printk("block error %d for op %d\n", rsp->status, rsp->operation);

@@ -343,29 +345,30 @@ moretodo:
         case BLKIF_OP_WRITE:
         {
             struct blkfront_aiocb *aiocbp = (void*) (uintptr_t) rsp->id;
+            int status = rsp->status;
             int j;
 
             for (j = 0; j < aiocbp->n; j++)
                 gnttab_end_access(aiocbp->gref[j]);
 
+            dev->ring.rsp_cons = ++cons;
             /* Nota: callback frees aiocbp itself */
-            aiocbp->aio_cb(aiocbp, rsp->status ? -EIO : 0);
+            aiocbp->aio_cb(aiocbp, status ? -EIO : 0);
+            if (dev->ring.rsp_cons != cons)
+                /* We reentered, we must not continue here */
+                goto out;
             break;
         }
+        default:
+            printk("unrecognized block operation %d response\n", rsp->operation);
         case BLKIF_OP_WRITE_BARRIER:
         case BLKIF_OP_FLUSH_DISKCACHE:
-            break;
-        default:
-            printk("unrecognized block operation %d response\n", rsp->operation);
+            dev->ring.rsp_cons = ++cons;
             break;
         }
+    }
 
-	nr_consumed++;
-	++cons;
-    }
-    dev->ring.rsp_cons = cons;
-
-    int more;
+out:
     RING_FINAL_CHECK_FOR_RESPONSES(&dev->ring, more);
     if (more) goto moretodo;
extras/mini-os/minios.mk +4 -4
diff -r 91c4e704169d extras/mini-os/minios.mk
@@ -16,10 +16,10 @@ DEF_LDFLAGS =
 
 ifeq ($(debug),y)
 DEF_CFLAGS += -g
-#DEF_CFLAGS += MM_DEBUG
-#DEF_CFLAGS += FS_DEBUG
-#DEF_CFLAGS += LIBC_DEBUG
-#DEF_CFLAGS += GNT_DEBUG
+#DEF_CFLAGS += -DMM_DEBUG
+#DEF_CFLAGS += -DFS_DEBUG
+#DEF_CFLAGS += -DLIBC_DEBUG
+DEF_CFLAGS += -DGNT_DEBUG
 else
 DEF_CFLAGS += -O3
 endif
author
#2 lore raw
Quoted 1 line · Samuel Thibault, le Sat 05 Apr 2008 14:31:08 +0200, a écrit :

That patch didn't get apply, is there any problem with it?

Oops sorry, discard that, I was looking at a non-updated repository.

Samuel