Re: [win-pv-devel] [PATCH 1/2] Destroy all handles on FdoD0ToD3

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 08 September 2017 06:29
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 1/2] Destroy all handles on FdoD0ToD3
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Since the StreamWorker holds a reference to the XENBUS_CONS interface,
> xenbus will BUG_ON if the handles are not cleaned up before power down.
> The service should close all handles in response to a
> DBT_DEVICEQUERYREMOVE
> notification, but this may not be issued on system power down.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>  src/xencons/fdo.c | 68
> +++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 61 insertions(+), 7 deletions(-)
> 
> diff --git a/src/xencons/fdo.c b/src/xencons/fdo.c
> index 5992776..1e660ea 100644
> --- a/src/xencons/fdo.c
> +++ b/src/xencons/fdo.c
> @@ -902,6 +902,11 @@ fail1:
>      return status;
>  }
> 
> +static VOID
> +FdoDestroyAllHandles(
> +    IN  PXENCONS_FDO    Fdo
> +    );
> +

Please re-order functions to avoid the need for function prototypes for static 
functions.

>  // This function must not touch pageable code or data
>  static DECLSPEC_NOINLINE VOID
>  FdoD0ToD3(
> @@ -920,6 +925,8 @@ FdoD0ToD3(
>  #pragma prefast(suppress:28123)
>      (VOID) IoSetDeviceInterfaceState(&Dx->Link, FALSE);
> 
> +    FdoDestroyAllHandles(Fdo);
> +
>      PowerState.DeviceState = PowerDeviceD3;
>      PoSetPowerState(Fdo->Dx->DeviceObject,
>                      DevicePowerState,
> @@ -2175,6 +2182,23 @@ fail1:
>      return NULL;
>  }
> 
> +static FORCEINLINE VOID
> +__FdoDestroyHandle(
> +    IN  PXENCONS_FDO    Fdo,
> +    IN  PFDO_HANDLE     Handle
> +    )
> +{
> +    RtlZeroMemory(&Handle->ListEntry, sizeof (LIST_ENTRY));
> +
> +    StreamDestroy(Handle->Stream);
> +    Handle->Stream = NULL;
> +
> +    Handle->FileObject = NULL;
> +
> +    ASSERT(IsZeroMemory(Handle, sizeof (FDO_HANDLE)));
> +    __FdoFree(Handle);
> +}
> +
>  static VOID
>  FdoDestroyHandle(
>      IN  PXENCONS_FDO    Fdo,
> @@ -2187,17 +2211,47 @@ FdoDestroyHandle(
>      RemoveEntryList(&Handle->ListEntry);
>      KeReleaseSpinLock(&Fdo->HandleLock, Irql);
> 
> -    RtlZeroMemory(&Handle->ListEntry, sizeof (LIST_ENTRY));
> -
>      Trace("%p\n", Handle->FileObject);

Move this Trace statement into the inline function, then you can get rid of the 
one in FdoDestroyAllHandles().

> 
> -    StreamDestroy(Handle->Stream);
> -    Handle->Stream = NULL;
> +    __FdoDestroyHandle(Fdo, Handle);
> +}
> 
> -    Handle->FileObject = NULL;
> +static VOID
> +FdoDestroyAllHandles(
> +    IN  PXENCONS_FDO    Fdo
> +    )
> +{
> +    KIRQL               Irql;
> +    LIST_ENTRY          ListHead;
> +    PLIST_ENTRY         ListEntry;
> +    PFDO_HANDLE         Handle;
> 
> -    ASSERT(IsZeroMemory(Handle, sizeof (FDO_HANDLE)));
> -    __FdoFree(Handle);
> +    InitializeListHead(&ListHead);
> +
> +    KeAcquireSpinLock(&Fdo->HandleLock, &Irql);
> +
> +    ListEntry = Fdo->HandleList.Flink;
> +    if (!IsListEmpty(&Fdo->HandleList)) {
> +        RemoveEntryList(&Fdo->HandleList);
> +        InitializeListHead(&Fdo->HandleList);
> +        AppendTailList(&ListHead, ListEntry);
> +    }
> +
> +    KeReleaseSpinLock(&Fdo->HandleLock, Irql);
> +
> +    for (;;) {

while (!IsListEmpty(&ListHead))?

Also s/ListHead/List please.

  Paul

> +        ListEntry = RemoveHeadList(&ListHead);
> +        if (ListEntry == &ListHead)
> +            break;
> +
> +        Handle = CONTAINING_RECORD(ListEntry,
> +                                   FDO_HANDLE,
> +                                   ListEntry);
> +
> +        Trace("%p\n", Handle->FileObject);
> +
> +        __FdoDestroyHandle(Fdo, Handle);
> +    }
>  }
> 
>  static DECLSPEC_NOINLINE NTSTATUS
> --
> 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