Re: [win-pv-devel] [PATCH 3/3] Return more error codes from Inflate/Deflate

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx [mailto:win-pv-devel-
> bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of Owen Smith
> Sent: 15 December 2015 11:30
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith
> Subject: [win-pv-devel] [PATCH 3/3] Return more error codes from
> Inflate/Deflate
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/xenbus/balloon.c | 69 ++++++++++++++++++++++++++++++++++-------
> -----------
>  1 file changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/src/xenbus/balloon.c b/src/xenbus/balloon.c
> index e2ce8de..5ef30a1 100644
> --- a/src/xenbus/balloon.c
> +++ b/src/xenbus/balloon.c
> @@ -590,7 +590,7 @@ BalloonLowMemory(
>      return (status == STATUS_SUCCESS) ? TRUE : FALSE;
>  }
> 
> -static BOOLEAN
> +static NTSTATUS
>  BalloonDeflate(
>      IN  PXENBUS_BALLOON_CONTEXT Context,
>      IN  ULONGLONG               Requested
> @@ -598,28 +598,29 @@ BalloonDeflate(
>  {
>      LARGE_INTEGER               Start;
>      LARGE_INTEGER               End;
> -    BOOLEAN                     Abort;
> +    NTSTATUS                    status;
>      ULONGLONG                   Count;
>      ULONGLONG                   TimeDelta;
> 
> +    status = STATUS_UNSUCCESSFUL;
>      if (Context->FIST.Deflation)
> -        return TRUE;
> +        goto done;
> 
>      Info("====> %llu page(s)\n", Requested);
> 
>      KeQuerySystemTime(&Start);
> 
>      Count = 0;
> -    Abort = FALSE;
> +    status = STATUS_SUCCESS;
> 
> -    while (Count < Requested && !Abort) {
> +    while (Count < Requested && NT_SUCCESS(status)) {
>          ULONG   ThisTime = (ULONG)__min(Requested - Count,
> XENBUS_BALLOON_PFN_ARRAY_SIZE);
>          ULONG   Populated;
>          ULONG   Freed;
> 
>          Populated = BalloonPopulatePfnArray(Context, ThisTime);
>          if (Populated < ThisTime)
> -            Abort = TRUE;
> +            status = STATUS_RETRY;
> 
>          Freed = BalloonFreePfnArray(Context, Populated, TRUE);
>          ASSERT(Freed == Populated);
> @@ -634,10 +635,11 @@ BalloonDeflate(
>      Info("<==== %llu page(s) in %llums\n", Count, TimeDelta);
>      Context->Size -= Count;
> 
> -    return Abort;
> +done:
> +    return status;
>  }
> 
> -static BOOLEAN
> +static NTSTATUS
>  BalloonInflate(
>      IN  PXENBUS_BALLOON_CONTEXT Context,
>      IN  ULONGLONG               Requested
> @@ -645,23 +647,26 @@ BalloonInflate(
>  {
>      LARGE_INTEGER               Start;
>      LARGE_INTEGER               End;
> -    BOOLEAN                     Abort;
> +    NTSTATUS                    status;
>      ULONGLONG                   Count;
>      ULONGLONG                   TimeDelta;
> 
> +    status = STATUS_UNSUCCESSFUL;
>      if (Context->FIST.Inflation)
> -        return TRUE;
> +        goto done;
> +
> +    status = STATUS_NO_MEMORY;
>      if (BalloonLowMemory(Context))
> -        return TRUE;
> +        goto done;

Ah. Forget my comment on patch #2 then.

> 
>      Info("====> %llu page(s)\n", Requested);
> 
>      KeQuerySystemTime(&Start);
> 
>      Count = 0;
> -    Abort = FALSE;
> +    status = STATUS_SUCCESS;
> 
> -    while (Count < Requested && !Abort) {
> +    while (Count < Requested && NT_SUCCESS(status)) {
>          ULONG   ThisTime = (ULONG)__min(Requested - Count,
> XENBUS_BALLOON_PFN_ARRAY_SIZE);
>          ULONG   Allocated;
>          BOOLEAN Slow;
> @@ -669,7 +674,7 @@ BalloonInflate(
> 
>          Allocated = BalloonAllocatePfnArray(Context, ThisTime, &Slow);
>          if (Allocated < ThisTime || Slow)
> -            Abort = TRUE;
> +            status = STATUS_RETRY;
> 
>          Released = BalloonReleasePfnArray(Context, Allocated);
> 
> @@ -685,7 +690,7 @@ BalloonInflate(
>          }
> 
>          if (Released == 0)
> -            Abort = TRUE;
> +            status = STATUS_RETRY;
> 
>          Count += Released;
>      }
> @@ -697,7 +702,8 @@ BalloonInflate(
>      Info("<==== %llu page(s) in %llums\n", Count, TimeDelta);
>      Context->Size += Count;
> 
> -    return Abort;
> +done:
> +    return status;
>  }
> 
>  static VOID
> @@ -747,6 +753,20 @@ BalloonGetFISTEntries(
>          Warning("deflation disallowed\n");
>  }
> 
> +static FORCEINLINE PCHAR

Should be 'const CHAR *'

> +__BalloonStatus(
> +    IN  NTSTATUS    status
> +    )
> +{
> +    switch (status) {
> +    case STATUS_SUCCESS:        return "";
> +    case STATUS_UNSUCCESSFUL:   return " [FIST]";
> +    case STATUS_RETRY:          return " [RETRY]";
> +    case STATUS_NO_MEMORY:      return " [LOW_MEM]";
> +    default:                    return " [UNKNOWN]";
> +    }

I generally dislike putting things on a single line like this, but I can fix 
that up.

> +}
> +
>  NTSTATUS
>  BalloonAdjust(
>      IN  PINTERFACE          Interface,
> @@ -754,28 +774,29 @@ BalloonAdjust(
>      )
>  {
>      PXENBUS_BALLOON_CONTEXT Context = Interface->Context;
> -    BOOLEAN                 Abort;
> +    NTSTATUS                status;
> 
>      ASSERT3U(KeGetCurrentIrql(), <, DISPATCH_LEVEL);
> 
>      Info("====> (%llu page(s))\n", Context->Size);
> 
> -    Abort = FALSE;
> +    status = STATUS_SUCCESS;
> 
>      BalloonGetFISTEntries(Context);
> 
> -    while (Context->Size != Size && !Abort) {
> +    while (Context->Size != Size && NT_SUCCESS(status)) {
>          if (Size > Context->Size)
> -            Abort = BalloonInflate(Context, Size - Context->Size);
> +            status = BalloonInflate(Context, Size - Context->Size);
>          else if (Size < Context->Size)
> -            Abort = BalloonDeflate(Context, Context->Size - Size);
> +            status = BalloonDeflate(Context, Context->Size - Size);
>      }
> 
> -    Info("<==== (%llu page(s))%s\n",
> +    Info("<==== (%llu page(s))%s%s\n",
>           Context->Size,
> -         (Abort) ? " [ABORTED]" : "");
> +         NT_SUCCESS(status) ? "" : " [ABORTED]",
> +         __BalloonStatus(status));

We don't really need [ABORTED] any more since __BalloonStatus() will now 
provide a more specific message. I'll get rid of it.

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

> 
> -    return (Abort) ? STATUS_RETRY : STATUS_SUCCESS;
> +    return status;
>  }
> 
>  ULONGLONG
> --
> 1.9.4.msysgit.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel

_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel