[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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) {
         if (Srb->SrbStatus == SRB_STATUS_PENDING) {
             // SRB has not hit a failure condition (BLKIF_RSP_ERROR | 
             // 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 
             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



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.