Re: [win-pv-devel] [PATCH 1/2] rate limit BLKIF_OP failure log messages XSI-413

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
> Daniel Davis
> Sent: 15 July 2019 14:13
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Daniel Davis <daniel.davis@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 1/2] rate limit BLKIF_OP failure log messages 
> XSI-413
> 
> Signed-off-by Daniel Davis <daniel.davis@xxxxxxxxxx>
> 
> ---
>  src/xenvbd/ring.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index 93fd920..431e1b5 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -52,6 +52,7 @@
>  #include "debug.h"
>  #include "assert.h"
> 
> +

Extraneous whitespace.

>  #define XENVBD_MAX_RING_PAGE_ORDER  (4)
>  #define XENVBD_MAX_RING_PAGES       (1 << XENVBD_MAX_RING_PAGE_ORDER)
> 
> @@ -94,6 +95,7 @@ typedef struct _XENVBD_BLKIF_RING {
>      ULONG                           RequestsPushed;
>      ULONG                           ResponsesProcessed;
>      PXENBUS_DEBUG_CALLBACK          DebugCallback;
> +     LARGE_INTEGER                                   TimeOfLastErrorLog;

There appears to be whitespace damage here. Have you got hard-tabs turned off?

>  } XENVBD_BLKIF_RING, *PXENVBD_BLKIF_RING;
> 
>  typedef enum _XENVBD_STAT {
> @@ -1213,12 +1215,22 @@ __BlkifRingCompleteResponse(
> 
>      case BLKIF_RSP_ERROR:
>      default:
> -        Warning("Target[%u][%u] : %s BLKIF_RSP_ERROR\n",
> -                FrontendGetTargetId(Frontend),
> -                BlkifRing->Index,
> -                __BlkifOperationName(Request->Operation));
> -        Srb->SrbStatus = SRB_STATUS_ERROR;
> -        break;
> +     {

Style. I think prevailing style has the opening brace on the last 'case' line.

> +             LARGE_INTEGER timeNow;
> +
> +             KeQuerySystemTime(&timeNow);

Style. We don't use the bizarre Windows not-quite-camel-case. 'TimeNow' or just 
plain 'Now' would be fine.

> +
> +             //if last log message was less than 10 seconds ago
> +             if (timeNow.QuadPart - BlkifRing->TimeOfLastErrorLog.QuadPart < 
> 100000000) {

I think 100000000 will truncate at 32 bits. Add a 'ull' suffix.

> +                     Warning("Target[%u][%u] : %s BLKIF_RSP_ERROR\n",
> +                             FrontendGetTargetId(Frontend),
> +                             BlkifRing->Index,
> +                             __BlkifOperationName(Request->Operation));
> +                     KeQuerySystemTime(&BlkifRing->TimeOfLastErrorLog);
> +             }
> +             Srb->SrbStatus = SRB_STATUS_ERROR;
> +             break;
> +             }
>      }
> 
>      BlkifRingPutRequest(BlkifRing, Request);
> @@ -1229,6 +1241,7 @@ __BlkifRingCompleteResponse(
>      }
>  }
> 
> +

Another stray whitespace change.

  Paul

>  static FORCEINLINE BOOLEAN
>  BlkifRingPoll(
>      IN  PXENVBD_BLKIF_RING  BlkifRing
> --
> 2.22.0.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