Re: [win-pv-devel] [PATCH 1/4] Rationalize WasQueued semantics

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 09 April 2018 09:48
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 1/4] Rationalize WasQueued semantics
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Several functions return values were inverted in the call stack,
> Fix the meaning of TRUE/FALSE all through the call stack for submitting
> requests
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

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

> ---
>  src/xenvbd/ring.c   | 16 ++++++++++++----
>  src/xenvbd/ring.h   |  2 +-
>  src/xenvbd/target.c | 28 +++++++++++-----------------
>  3 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
> index f612fc7..5af1a69 100644
> --- a/src/xenvbd/ring.c
> +++ b/src/xenvbd/ring.c
> @@ -2120,20 +2120,28 @@ RingTrigger(
>                    Ring->Channel);
>  }
> 
> -VOID
> +BOOLEAN
>  RingQueueRequest(
>      IN  PXENVBD_RING    Ring,
>      IN  PXENVBD_SRBEXT  SrbExt
>      )
>  {
> -    QueueAppend(&Ring->FreshSrbs,
> -                &SrbExt->ListEntry);
> +    PSCSI_REQUEST_BLOCK Srb = SrbExt->Srb;
> 
>      if (!Ring->Enabled)
> -        return;
> +        goto fail1;
> +
> +    QueueAppend(&Ring->FreshSrbs,
> +                &SrbExt->ListEntry);
> 
>      if (KeInsertQueueDpc(&Ring->Dpc, NULL, NULL))
>           ++Ring->Dpcs;
> +
> +    return TRUE;
> +
> +fail1:
> +    Srb->SrbStatus = SRB_STATUS_BUSY;
> +    return FALSE;
>  }
> 
>  VOID
> diff --git a/src/xenvbd/ring.h b/src/xenvbd/ring.h
> index 81f4e99..9b8a587 100644
> --- a/src/xenvbd/ring.h
> +++ b/src/xenvbd/ring.h
> @@ -79,7 +79,7 @@ RingTrigger(
>      IN  PXENVBD_RING    Ring
>      );
> 
> -extern VOID
> +extern BOOLEAN
>  RingQueueRequest(
>      IN  PXENVBD_RING    Ring,
>      IN  PXENVBD_SRBEXT  SrbExt
> diff --git a/src/xenvbd/target.c b/src/xenvbd/target.c
> index f342161..54ffb93 100644
> --- a/src/xenvbd/target.c
> +++ b/src/xenvbd/target.c
> @@ -242,8 +242,7 @@ TargetReadWrite(
>          goto fail4;
> 
>      Srb->SrbStatus = SRB_STATUS_PENDING;
> -    RingQueueRequest(Ring, SrbExt);
> -    return FALSE;
> +    return RingQueueRequest(Ring, SrbExt);
> 
>  fail4:
>       Error("fail4\n");
> @@ -253,7 +252,7 @@ fail2:
>      Error("fail2\n");
>  fail1:
>      Error("fail1\n");
> -    return TRUE;
> +    return FALSE; // not-queued
>  }
> 
>  static DECLSPEC_NOINLINE BOOLEAN
> @@ -279,18 +278,17 @@ TargetSyncCache(
>          goto succeed;
> 
>      Srb->SrbStatus = SRB_STATUS_PENDING;
> -    RingQueueRequest(Ring, SrbExt);
> -    return FALSE;
> +    return RingQueueRequest(Ring, SrbExt);
> 
>  succeed:
>      Srb->SrbStatus = SRB_STATUS_SUCCESS;
> -    return TRUE;
> +    return FALSE; // not-queued
> 
>  fail2:
>      Error("fail2\n");
>  fail1:
>      Error("fail1\n");
> -    return TRUE;
> +    return FALSE; // not-queued
>  }
> 
>  static DECLSPEC_NOINLINE BOOLEAN
> @@ -314,18 +312,17 @@ TargetUnmap(
>          goto succeed;
> 
>      Srb->SrbStatus = SRB_STATUS_PENDING;
> -    RingQueueRequest(Ring, SrbExt);
> -    return FALSE;
> +    return RingQueueRequest(Ring, SrbExt);
> 
>  succeed:
>      Srb->SrbStatus = SRB_STATUS_SUCCESS;
> -    return TRUE;
> +    return FALSE; // not-queued
> 
>  fail2:
>      Error("fail2\n");
>  fail1:
>      Error("fail1\n");
> -    return TRUE;
> +    return FALSE; // not-queued
>  }
> 
>  static FORCEINLINE VOID
> @@ -901,18 +898,15 @@ TargetStartIo(
>      switch (Operation) {
>      case SCSIOP_READ:
>      case SCSIOP_WRITE:
> -        if (!TargetReadWrite(Target, Srb))
> -            WasQueued = TRUE;
> +        WasQueued = TargetReadWrite(Target, Srb);
>          break;
> 
>      case SCSIOP_UNMAP:
> -        if (!TargetUnmap(Target, Srb))
> -            WasQueued = TRUE;
> +        WasQueued = TargetUnmap(Target, Srb);
>          break;
> 
>      case SCSIOP_SYNCHRONIZE_CACHE:
> -        if (!TargetSyncCache(Target, Srb))
> -            WasQueued = TRUE;
> +        WasQueued = TargetSyncCache(Target, Srb);
>          break;
> 
>      case SCSIOP_INQUIRY:
> --
> 2.8.3
> 
> 
> _______________________________________________
> 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