Re: [win-pv-devel] [PATCH] Add DPC timeout check

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 16 August 2017 16:34
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH] Add DPC timeout check
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Check the DPC has not exceeded 1/2 its alloted time every
> * 1/4 ring of responses processed
> * all outstanding prepared requests submitted
> * 1 queued SRB prepared
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Acked-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  src/xenvbd/blockring.c | 12 ++++++---
>  src/xenvbd/blockring.h |  2 +-
>  src/xenvbd/frontend.c  | 10 +++++---
>  src/xenvbd/frontend.h  |  2 +-
>  src/xenvbd/notifier.c  | 66
> ++++++++++++++++++++++++++++++++++++++++++++++----
>  src/xenvbd/target.c    | 13 ++++++++--
>  src/xenvbd/target.h    |  2 +-
>  7 files changed, 91 insertions(+), 16 deletions(-)
> 
> diff --git a/src/xenvbd/blockring.c b/src/xenvbd/blockring.c
> index e00b64a..24e47c0 100644
> --- a/src/xenvbd/blockring.c
> +++ b/src/xenvbd/blockring.c
> @@ -506,12 +506,13 @@ BlockRingDebugCallback(
>      BlockRing->Submitted = BlockRing->Received = 0;
>  }
> 
> -VOID
> +BOOLEAN
>  BlockRingPoll(
>      IN  PXENVBD_BLOCKRING           BlockRing
>      )
>  {
>      PXENVBD_TARGET Target = FrontendGetTarget(BlockRing->Frontend);
> +    BOOLEAN         Retry = FALSE;
> 
>      ASSERT3U(KeGetCurrentIrql(), ==, DISPATCH_LEVEL);
>      KeAcquireSpinLockAtDpcLevel(&BlockRing->Lock);
> @@ -532,10 +533,10 @@ BlockRingPoll(
> 
>          KeMemoryBarrier();
> 
> -        if (rsp_cons == rsp_prod)
> +        if (rsp_cons == rsp_prod || Retry)
>              break;
> 
> -        while (rsp_cons != rsp_prod) {
> +        while (rsp_cons != rsp_prod && !Retry) {
>              blkif_response_t*   Response;
>              ULONG               Tag;
> 
> @@ -548,6 +549,9 @@ BlockRingPoll(
>              }
> 
>              RtlZeroMemory(Response, sizeof(union blkif_sring_entry));
> +
> +            if (rsp_cons - BlockRing->FrontRing.rsp_cons >
> RING_SIZE(&BlockRing->FrontRing) / 4)
> +                Retry = TRUE;
>          }
> 
>          KeMemoryBarrier();
> @@ -558,6 +562,8 @@ BlockRingPoll(
> 
>  done:
>      KeReleaseSpinLockFromDpcLevel(&BlockRing->Lock);
> +
> +    return Retry;
>  }
> 
>  BOOLEAN
> diff --git a/src/xenvbd/blockring.h b/src/xenvbd/blockring.h
> index 1117d73..98fe278 100644
> --- a/src/xenvbd/blockring.h
> +++ b/src/xenvbd/blockring.h
> @@ -83,7 +83,7 @@ BlockRingDebugCallback(
>      IN  PXENBUS_DEBUG_INTERFACE     Debug
>      );
> 
> -extern VOID
> +extern BOOLEAN
>  BlockRingPoll(
>      IN  PXENVBD_BLOCKRING           BlockRing
>      );
> diff --git a/src/xenvbd/frontend.c b/src/xenvbd/frontend.c
> index c909ea8..2ec082d 100644
> --- a/src/xenvbd/frontend.c
> +++ b/src/xenvbd/frontend.c
> @@ -326,13 +326,17 @@ out:
> 
> 
> //=========================================================
> ====================
>  __drv_requiresIRQL(DISPATCH_LEVEL)
> -VOID
> +BOOLEAN
>  FrontendNotifyResponses(
>      __in  PXENVBD_FRONTEND        Frontend
>      )
>  {
> -    BlockRingPoll(Frontend->BlockRing);
> -    TargetSubmitRequests(Frontend->Target);
> +    BOOLEAN     Retry = FALSE;
> +
> +    Retry |= BlockRingPoll(Frontend->BlockRing);
> +    Retry |= TargetSubmitRequests(Frontend->Target);
> +
> +    return Retry;
>  }
> 
> 
> //=========================================================
> ====================
> diff --git a/src/xenvbd/frontend.h b/src/xenvbd/frontend.h
> index 8a6cc04..4490aeb 100644
> --- a/src/xenvbd/frontend.h
> +++ b/src/xenvbd/frontend.h
> @@ -149,7 +149,7 @@ FrontendWriteUsage(
> 
>  // Ring
>  __drv_requiresIRQL(DISPATCH_LEVEL)
> -extern VOID
> +extern BOOLEAN
>  FrontendNotifyResponses(
>      __in  PXENVBD_FRONTEND        Frontend
>      );
> diff --git a/src/xenvbd/notifier.c b/src/xenvbd/notifier.c
> index a87c3bd..22bfe00 100644
> --- a/src/xenvbd/notifier.c
> +++ b/src/xenvbd/notifier.c
> @@ -50,6 +50,8 @@ struct _XENVBD_NOTIFIER {
>      ULONG                           NumInts;
>      ULONG                           NumDpcs;
>      KDPC                            Dpc;
> +    KDPC                            TimerDpc;
> +    KTIMER                          Timer;
>  };
> 
>  #define NOTIFIER_POOL_TAG           'yfNX'
> @@ -95,6 +97,37 @@ NotifierInterrupt(
>      return TRUE;
>  }
> 
> +static FORCEINLINE BOOLEAN
> +__NotifierDpcTimeout(
> +    IN  PXENVBD_NOTIFIER        Notifier
> +    )
> +{
> +    KDPC_WATCHDOG_INFORMATION   Watchdog;
> +    NTSTATUS                    status;
> +
> +    UNREFERENCED_PARAMETER(Notifier);
> +
> +    RtlZeroMemory(&Watchdog, sizeof (Watchdog));
> +
> +    status = KeQueryDpcWatchdogInformation(&Watchdog);
> +    ASSERT(NT_SUCCESS(status));
> +
> +    if (Watchdog.DpcTimeLimit == 0 ||
> +        Watchdog.DpcWatchdogLimit == 0)
> +        return FALSE;
> +
> +    if (Watchdog.DpcTimeCount > (Watchdog.DpcTimeLimit / 2) &&
> +        Watchdog.DpcWatchdogCount > (Watchdog.DpcWatchdogLimit / 2))
> +        return FALSE;
> +
> +    return TRUE;
> +}
> +
> +#define TIME_US(_us)        ((_us) * 10)
> +#define TIME_MS(_ms)        (TIME_US((_ms) * 1000))
> +#define TIME_S(_s)          (TIME_MS((_s) * 1000))
> +#define TIME_RELATIVE(_t)   (-(_t))
> +
>  KDEFERRED_ROUTINE NotifierDpc;
> 
>  VOID
> @@ -116,12 +149,25 @@ NotifierDpc(
>      if (!Notifier->Connected)
>          return;
> 
> -    FrontendNotifyResponses(Notifier->Frontend);
> +    for (;;) {
> +        if (!FrontendNotifyResponses(Notifier->Frontend)) {
> +            XENBUS_EVTCHN(Unmask,
> +                          &Notifier->EvtchnInterface,
> +                          Notifier->Channel,
> +                          FALSE);
> +            break;
> +        }
> +        if (__NotifierDpcTimeout(Notifier)) {
> +            LARGE_INTEGER   Delay;
> 
> -    XENBUS_EVTCHN(Unmask,
> -                  &Notifier->EvtchnInterface,
> -                  Notifier->Channel,
> -                  FALSE);
> +            Delay.QuadPart = TIME_RELATIVE(TIME_US(100));
> +
> +            KeSetTimer(&Notifier->Timer,
> +                       Delay,
> +                       &Notifier->TimerDpc);
> +            break;
> +        }
> +    }
>  }
> 
>  NTSTATUS
> @@ -136,6 +182,8 @@ NotifierCreate(
> 
>      (*Notifier)->Frontend = Frontend;
>      KeInitializeDpc(&(*Notifier)->Dpc, NotifierDpc, *Notifier);
> +    KeInitializeDpc(&(*Notifier)->TimerDpc, NotifierDpc, *Notifier);
> +    KeInitializeTimer(&(*Notifier)->Timer);
> 
>      return STATUS_SUCCESS;
> 
> @@ -150,6 +198,8 @@ NotifierDestroy(
>  {
>      Notifier->Frontend = NULL;
>      RtlZeroMemory(&Notifier->Dpc, sizeof(KDPC));
> +    RtlZeroMemory(&Notifier->TimerDpc, sizeof(KDPC));
> +    RtlZeroMemory(&Notifier->Timer, sizeof(KTIMER));
> 
>      ASSERT(IsZeroMemory(Notifier, sizeof(XENVBD_NOTIFIER)));
> 
> @@ -252,6 +302,12 @@ NotifierDisable(
>      ASSERT(Notifier->Enabled == TRUE);
> 
>      Notifier->Enabled = FALSE;
> +
> +    //
> +    // No new timers can be scheduled once Enabled goes to FALSE.
> +    // Cancel any existing ones.
> +    //
> +    (VOID) KeCancelTimer(&Notifier->Timer);
>  }
> 
>  VOID
> diff --git a/src/xenvbd/target.c b/src/xenvbd/target.c
> index ffffa3e..697f309 100644
> --- a/src/xenvbd/target.c
> +++ b/src/xenvbd/target.c
> @@ -1254,11 +1254,13 @@ BlkifOperationName(
>      }
>  }
> 
> -VOID
> +BOOLEAN
>  TargetSubmitRequests(
>      IN  PXENVBD_TARGET  Target
>      )
>  {
> +    BOOLEAN             Retry = FALSE;
> +
>      for (;;) {
>          // submit all prepared requests (0 or more requests)
>          // return TRUE if submitted 0 or more requests from prepared queue
> @@ -1271,10 +1273,17 @@ TargetSubmitRequests(
>          // return FALSE if prepare failed or fresh queue empty
>          if (!TargetPrepareFresh(Target))
>              break;
> +
> +        // back off, check DPC timeout and try again
> +        Retry = TRUE;
> +        break;
>      }
> 
>      // if no requests/SRBs outstanding, complete any shutdown SRBs
> -    TargetCompleteShutdown(Target);
> +    if (!Retry)
> +        TargetCompleteShutdown(Target);
> +
> +    return Retry;
>  }
> 
>  VOID
> diff --git a/src/xenvbd/target.h b/src/xenvbd/target.h
> index 1e2b3ed..b790eb4 100644
> --- a/src/xenvbd/target.h
> +++ b/src/xenvbd/target.h
> @@ -93,7 +93,7 @@ TargetSetDeviceObject(
>      IN  PDEVICE_OBJECT  DeviceObject
>      );
> 
> -extern VOID
> +extern BOOLEAN
>  TargetSubmitRequests(
>      IN  PXENVBD_TARGET  Target
>      );
> --
> 2.8.3
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel