Re: [win-pv-devel] [PATCH 4/4] Rework BlkifRingDisable

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Owen Smith
> Sent: 16 September 2019 16:18
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 4/4] Rework BlkifRingDisable
> 
> Clean up all prepared and submitted requests when the ring is disabled,
> so that outstanding SRBs are returned to storport for queueing. This is
> especially important on the return from suspend path, as the ring is no
> longer valid, and any submitted requests would be lost and trigger a
> storport target reset.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/xenvbd/ring.c | 62 
> +++++++++++++++++++++----------------------------------
>  1 file changed, 23 insertions(+), 39 deletions(-)
> 
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index d5db1da..5323243 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -1242,16 +1242,17 @@ BlkifRingPoll(
> 
>              rsp = RING_GET_RESPONSE(&BlkifRing->Front, rsp_cons);
>              rsp_cons++;
> -            BlkifRing->ResponsesProcessed++;
> 
>              BlkifRing->Stopped = FALSE;
> 
>              Request = __BlkifRingGetSubmittedRequest(BlkifRing,
>                                                       rsp->id);
> -            if (Request != NULL)
> +            if (Request != NULL) {
> +                BlkifRing->ResponsesProcessed++;
>                  __BlkifRingCompleteResponse(BlkifRing,
>                                              Request,
>                                              rsp->status);
> +            }

Still don't understand why this is conditional, but if it needs to be then then 
moving the counter bump should surely be in the previous patch.

  Paul

> 
>              if (rsp_cons - BlkifRing->Front.rsp_cons > 
> XENVBD_BATCH(BlkifRing))
>                  Retry = TRUE;
> @@ -2043,58 +2044,41 @@ BlkifRingDisable(
>      IN  PXENVBD_BLKIF_RING  BlkifRing
>      )
>  {
> -    PXENVBD_RING            Ring = BlkifRing->Ring;
> -    ULONG                   Attempt;
> -
>      Trace("====> %u\n", BlkifRing->Index);
> 
>      __BlkifRingAcquireLock(BlkifRing);
>      ASSERT(BlkifRing->Enabled);
> 
> -    // Discard any pending requests
> +    BlkifRing->Enabled = FALSE;
> +
>      for (;;) {
> -        PLIST_ENTRY         ListEntry;
> -        PXENVBD_REQUEST     Request;
> -        PXENVBD_SRBEXT      SrbExt;
> -        PSCSI_REQUEST_BLOCK Srb;
> +        PLIST_ENTRY ListEntry;
> +        PXENVBD_REQUEST Request;
> 
> -        ListEntry = RemoveHeadList(&BlkifRing->PreparedQueue);
> -        if (ListEntry  == &BlkifRing->PreparedQueue)
> +        ListEntry = RemoveHeadList(&BlkifRing->SubmittedList);
> +        if (ListEntry == &BlkifRing->SubmittedList)
>              break;
> 
> -        Request = CONTAINING_RECORD(ListEntry,
> -                                    XENVBD_REQUEST,
> -                                    ListEntry);
> -        SrbExt = Request->SrbExt;
> -        Srb = SrbExt->Srb;
> -        Srb->SrbStatus = SRB_STATUS_ABORTED;
> -        Srb->ScsiStatus = 0x40; // SCSI_ABORTED
> -
> -        BlkifRingPutRequest(BlkifRing, Request);
> -
> -        if (InterlockedDecrement(&SrbExt->RequestCount) == 0)
> -            __BlkifRingCompleteSrb(BlkifRing, SrbExt);
> +        Request = CONTAINING_RECORD(ListEntry, XENVBD_REQUEST, ListEntry);
> +        BlkifRing->ResponsesProcessed++;
> +        __BlkifRingCompleteResponse(BlkifRing, Request, BLKIF_RSP_ERROR);
>      }
> 
> -    Attempt = 0;
> -    ASSERT3U(BlkifRing->RequestsPushed, == , BlkifRing->RequestsPosted);
> -    while (BlkifRing->ResponsesProcessed != BlkifRing->RequestsPushed) {
> -        Attempt++;
> -        ASSERT(Attempt < 100);
> -
> -        // Try to move things along
> -        __BlkifRingSend(BlkifRing);
> -        (VOID)BlkifRingPoll(BlkifRing);
> +    for (;;) {
> +        PLIST_ENTRY ListEntry;
> +        PXENVBD_REQUEST Request;
> 
> -        // We are waiting for a watch event at DISPATCH_LEVEL so
> -        // it is our responsibility to poll the store ring.
> -        XENBUS_STORE(Poll,
> -                     &Ring->StoreInterface);
> +        ListEntry = RemoveHeadList(&BlkifRing->PreparedQueue);
> +        if (ListEntry == &BlkifRing->PreparedQueue)
> +            break;
> 
> -        KeStallExecutionProcessor(1000);    // 1ms
> +        Request = CONTAINING_RECORD(ListEntry, XENVBD_REQUEST, ListEntry);
> +        // Dont increment ResponsesProcessed, as this is a faked response
> +        __BlkifRingCompleteResponse(BlkifRing, Request, BLKIF_RSP_ERROR);
>      }
> 
> -    BlkifRing->Enabled = FALSE;
> +    BlkifRing->Stopped = FALSE;
> +
>      __BlkifRingReleaseLock(BlkifRing);
> 
>      Trace("<==== %u\n", BlkifRing->Index);
> --
> 2.16.2.windows.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel