[win-pv-devel] [PATCH 2/2] Fix race calculating SrbExt->Count

[<owen.smith@xxxxxxxxxx>]

From: Owen Smith <owen.smith@xxxxxxxxxx>

It is possible under heavy loads for the backend to
start completing sub-requests of a Srb before the
SrbExt Count is set. This would leave the count unable
to reach 0 (as 1 or more requests are skipped by count
being overridden)

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/xenvbd/pdo.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/xenvbd/pdo.c b/src/xenvbd/pdo.c
index dd5b6ea..ee2a3cc 100644
--- a/src/xenvbd/pdo.c
+++ b/src/xenvbd/pdo.c
@@ -1169,6 +1169,7 @@ PrepareReadWrite(
     ULONG           SectorsLeft = Cdb_TransferBlock(Srb);
     LIST_ENTRY      List;
     XENVBD_SG_LIST  SGList;
+    ULONG           DebugCount;
 
     InitializeListHead(&List);
     SrbExt->Count = 0;
@@ -1185,6 +1186,7 @@ PrepareReadWrite(
         if (Request == NULL) 
             goto fail1;
         InsertTailList(&List, &Request->Entry);
+        InterlockedIncrement(&SrbExt->Count);
         
         Request->Srb    = Srb;
         MaxSegments = UseIndirect(Pdo, SectorsLeft);
@@ -1207,7 +1209,10 @@ PrepareReadWrite(
         SectorStart += SectorsDone;
     }
 
-    SrbExt->Count = PdoQueueRequestList(Pdo, &List);
+    DebugCount = PdoQueueRequestList(Pdo, &List);
+    if (DebugCount != (ULONG)SrbExt->Count) {
+        Trace("[%u] %d != %u\n", PdoGetTargetId(Pdo), SrbExt->Count, 
DebugCount);
+    }
     Srb->SrbStatus = SRB_STATUS_PENDING;
     return TRUE;
 
@@ -1215,6 +1220,7 @@ fail3:
 fail2:
 fail1:
     PdoCancelRequestList(Pdo, &List);
+    SrbExt->Count = 0;
     Srb->SrbStatus = SRB_STATUS_ERROR;
     return FALSE;
 }
@@ -1230,6 +1236,7 @@ PrepareSyncCache(
     PXENVBD_REQUEST     Request;
     LIST_ENTRY          List;
     UCHAR               Operation;
+    ULONG               DebugCount;
     
     if (FrontendGetDiskInfo(Pdo->Frontend)->FlushCache)
         Operation = BLKIF_OP_FLUSH_DISKCACHE;
@@ -1243,17 +1250,22 @@ PrepareSyncCache(
     if (Request == NULL)
         goto fail1;
     InsertTailList(&List, &Request->Entry);
+    InterlockedIncrement(&SrbExt->Count);
 
     Request->Srb        = Srb;
     Request->Operation  = Operation;
     Request->FirstSector = Cdb_LogicalBlock(Srb);
 
-    SrbExt->Count = PdoQueueRequestList(Pdo, &List);
+    DebugCount = PdoQueueRequestList(Pdo, &List);
+    if (DebugCount != (ULONG)SrbExt->Count) {
+        Trace("[%u] %d != %u\n", PdoGetTargetId(Pdo), SrbExt->Count, 
DebugCount);
+    }
     Srb->SrbStatus = SRB_STATUS_PENDING;
     return TRUE;
 
 fail1:
     PdoCancelRequestList(Pdo, &List);
+    SrbExt->Count = 0;
     Srb->SrbStatus = SRB_STATUS_ERROR;
     return FALSE;
 }
@@ -1270,6 +1282,7 @@ PrepareUnmap(
        ULONG               Count = 
_byteswap_ushort(*(PUSHORT)Unmap->BlockDescrDataLength) / 
sizeof(UNMAP_BLOCK_DESCRIPTOR);
     ULONG               Index;
     LIST_ENTRY          List;
+    ULONG               DebugCount;
 
     InitializeListHead(&List);
     SrbExt->Count = 0;
@@ -1282,6 +1295,7 @@ PrepareUnmap(
         if (Request == NULL)
             goto fail1;
         InsertTailList(&List, &Request->Entry);
+        InterlockedIncrement(&SrbExt->Count);
 
         Request->Srb            = Srb;
         Request->Operation      = BLKIF_OP_DISCARD;
@@ -1290,12 +1304,16 @@ PrepareUnmap(
         Request->Flags          = 0;
     }
 
-    SrbExt->Count = PdoQueueRequestList(Pdo, &List);
+    DebugCount = PdoQueueRequestList(Pdo, &List);
+    if (DebugCount != (ULONG)SrbExt->Count) {
+        Trace("[%u] %d != %u\n", PdoGetTargetId(Pdo), SrbExt->Count, 
DebugCount);
+    }
     Srb->SrbStatus = SRB_STATUS_PENDING;
     return TRUE;
 
 fail1:
     PdoCancelRequestList(Pdo, &List);
+    SrbExt->Count = 0;
     Srb->SrbStatus = SRB_STATUS_ERROR;
     return FALSE;
 }
@@ -1429,6 +1447,13 @@ PdoSubmitPrepared(
     )
 {
     PXENVBD_BLOCKRING   BlockRing = FrontendGetBlockRing(Pdo->Frontend);
+    if (PdoIsPaused(Pdo)) {
+        if (QueueCount(&Pdo->PreparedReqs))
+            Warning("Target[%d] : Paused, not submitting new requests (%u)\n",
+                    PdoGetTargetId(Pdo),
+                    QueueCount(&Pdo->PreparedReqs));
+        return FALSE;
+    }
 
     for (;;) {
         PXENVBD_REQUEST Request;
-- 
2.8.3


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel