[win-pv-devel] commit "Make sure we don't leave SRBs in state PENDING" introduces race

[Andreas Kinzler <ml-ak@xxxxxxxxx>]

Hello Paul,

     SrbExt->Count = PdoQueueRequestList(Pdo, &List);
+    Srb->SrbStatus = SRB_STATUS_PENDING;
     return TRUE;

doesn't everything you do after PdoQueueRequestList introduce a race, be it writing 
to SrbExt->Count or setting Srb->SrbStatus?

In PdoCompleteResponse

    if (InterlockedDecrement(&SrbExt->Count) == 0) {
--->race
         if (Srb->SrbStatus == SRB_STATUS_PENDING) {
             // SRB has not hit a failure condition (BLKIF_RSP_ERROR | 
BLKIF_RSP_EOPNOTSUPP)
             // from any of its responses. SRB must have succeeded
             Srb->SrbStatus = SRB_STATUS_SUCCESS;
             Srb->ScsiStatus = 0x00; // SCSI_GOOD
         } else {
             // Srb->SrbStatus has already been set by 1 or more requests with 
Status != BLKIF_RSP_OKAY
             Srb->ScsiStatus = 0x40; // SCSI_ABORTED
         }

So you may overwrite an already existing error condition.

I would really suggest to use my patch. So far load tests are finally stable 
with it.

Regards Andreas

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