Re: [win-pv-devel] [PATCH] Use a DPC per XENIFACE_EVTCHN_CONTEXT for signalling to user space

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx [mailto:win-pv-devel-
> bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of Paul Durrant
> Sent: 10 December 2015 12:45
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Paul Durrant; Rafal Wojdyla
> Subject: [win-pv-devel] [PATCH] Use a DPC per
> XENIFACE_EVTCHN_CONTEXT for signalling to user space
> 
> The scheme of using a DPC per CPU will not work when multiple event
> channels
> are in use since events may be lost when channels bound to the same CPU
> are simultaneously pending. The problem is avoided by simply having
> separate DPCs for each channel.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Rafal Wojdyla <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  src/xeniface/fdo.c          | 37 ++-----------------------------------
>  src/xeniface/fdo.h          |  1 -
>  src/xeniface/ioctl_evtchn.c | 12 +++++++++---
>  src/xeniface/ioctls.h       |  1 +
>  4 files changed, 12 insertions(+), 39 deletions(-)
> 
> diff --git a/src/xeniface/fdo.c b/src/xeniface/fdo.c
> index b23832d..fa14b5b 100644
> --- a/src/xeniface/fdo.c
> +++ b/src/xeniface/fdo.c
> @@ -2435,13 +2435,11 @@ FdoCreate(
>      )
>  {
>      PDEVICE_OBJECT      FunctionDeviceObject;
> -    PXENIFACE_DX          Dx;
> -    PXENIFACE_FDO         Fdo;
> +    PXENIFACE_DX        Dx;
> +    PXENIFACE_FDO       Fdo;
>      WCHAR               Name[MAXNAMELEN * sizeof (WCHAR)];
>      ULONG               Size;
>      NTSTATUS            status;
> -    ULONG               ProcessorCount;
> -    ULONG               Index;
> 
>  #pragma prefast(suppress:28197) // Possibly leaking memory
> 'FunctionDeviceObject'
>      status = IoCreateDevice(DriverObject,
> @@ -2587,25 +2585,6 @@ FdoCreate(
>      if (!NT_SUCCESS(status))
>          goto fail15;
> 
> -    ProcessorCount =
> KeQueryMaximumProcessorCountEx(ALL_PROCESSOR_GROUPS);
> -
> -    Fdo->EvtchnDpc = __FdoAllocate(sizeof (KDPC) * ProcessorCount);
> -
> -    status = STATUS_NO_MEMORY;
> -    if (Fdo->EvtchnDpc == NULL)
> -        goto fail16;
> -
> -    for (Index = 0; Index < ProcessorCount; Index++) {
> -        PROCESSOR_NUMBER ProcNumber;
> -
> -        status = KeGetProcessorNumberFromIndex(Index, &ProcNumber);
> -        ASSERT(NT_SUCCESS(status));
> -
> -        KeInitializeDpc(&Fdo->EvtchnDpc[Index], EvtchnNotificationDpc, NULL);
> -        status = KeSetTargetProcessorDpcEx(&Fdo->EvtchnDpc[Index],
> &ProcNumber);
> -        ASSERT(NT_SUCCESS(status));
> -    }
> -
>      Info("%p (%s)\n",
>           FunctionDeviceObject,
>           __FdoGetName(Fdo));
> @@ -2615,11 +2594,6 @@ FdoCreate(
> 
>      return STATUS_SUCCESS;
> 
> -fail16:
> -    Error("fail6\n");
> -
> -    RtlZeroMemory(&Fdo->IrpQueue, sizeof (IO_CSQ));
> -
>  fail15:
>      Error("fail15\n");
> 
> @@ -2731,7 +2705,6 @@ FdoDestroy(
>  {
>      PXENIFACE_DX          Dx = Fdo->Dx;
>      PDEVICE_OBJECT        FunctionDeviceObject = Dx->DeviceObject;
> -    ULONG                 ProcessorCount;
> 
>      ASSERT(IsListEmpty(&Dx->ListEntry));
>      ASSERT3U(Fdo->References, ==, 0);
> @@ -2745,12 +2718,6 @@ FdoDestroy(
> 
>      Dx->Fdo = NULL;
> 
> -    ProcessorCount =
> KeQueryMaximumProcessorCountEx(ALL_PROCESSOR_GROUPS);
> -    RtlZeroMemory(Fdo->EvtchnDpc, sizeof (KDPC) * ProcessorCount);
> -
> -    __FdoFree(Fdo->EvtchnDpc);
> -    Fdo->EvtchnDpc = NULL;
> -
>      RtlZeroMemory(&Fdo->GnttabCacheLock, sizeof (KSPIN_LOCK));
>      ASSERT(IsListEmpty(&Fdo->IrpList));
>      RtlZeroMemory(&Fdo->IrpList, sizeof (LIST_ENTRY));
> diff --git a/src/xeniface/fdo.h b/src/xeniface/fdo.h
> index 781b1cc..6cd628c 100644
> --- a/src/xeniface/fdo.h
> +++ b/src/xeniface/fdo.h
> @@ -89,7 +89,6 @@ typedef struct _XENIFACE_FDO {
> 
>      KSPIN_LOCK                      EvtchnLock;
>      LIST_ENTRY                      EvtchnList;
> -    PKDPC                           EvtchnDpc;
> 
>      KSPIN_LOCK                      GnttabCacheLock;
> 
> diff --git a/src/xeniface/ioctl_evtchn.c b/src/xeniface/ioctl_evtchn.c
> index 9c5af19..84b4871 100644
> --- a/src/xeniface/ioctl_evtchn.c
> +++ b/src/xeniface/ioctl_evtchn.c
> @@ -47,10 +47,10 @@ EvtchnNotificationDpc(
>      __in_opt  PVOID Argument2
>      )
>  {
> -    PXENIFACE_EVTCHN_CONTEXT Context = Argument1;
> +    PXENIFACE_EVTCHN_CONTEXT Context = _Context;
> 
>      UNREFERENCED_PARAMETER(Dpc);
> -    UNREFERENCED_PARAMETER(_Context);
> +    UNREFERENCED_PARAMETER(Argument1);
>      UNREFERENCED_PARAMETER(Argument2);
> 
>      ASSERT(Context != NULL);
> @@ -78,11 +78,13 @@ EvtchnInterruptHandler(
>      ULONG ProcIndex;
> 
>      UNREFERENCED_PARAMETER(Interrupt);
> +
>      ASSERT(Context != NULL);
> 
>      KeGetCurrentProcessorNumberEx(&ProcNumber);
>      ProcIndex = KeGetProcessorIndexFromNumber(&ProcNumber);
> -    if (!KeInsertQueueDpc(&Context->Fdo->EvtchnDpc[ProcIndex], Context,
> NULL)) {
> +
> +    if (!KeInsertQueueDpc(&Context->Dpc, Context, NULL)) {

Actually, I realise that arg1 can be NULL now too.

  Paul

>          XenIfaceDebugPrint(TRACE, "NOT INSERTED: Context %p, Port %lu, FO
> %p, Cpu %lu\n",
>                             Context, Context->LocalPort, Context->FileObject, 
> ProcIndex);
>      }
> @@ -189,6 +191,8 @@ IoctlEvtchnBindUnbound(
>      if (!NT_SUCCESS(status))
>          goto fail3;
> 
> +    KeInitializeDpc(&Context->Dpc, EvtchnNotificationDpc, Context);
> +
>      status = STATUS_UNSUCCESSFUL;
>      Context->Channel = XENBUS_EVTCHN(Open,
>                                       &Fdo->EvtchnInterface,
> @@ -280,6 +284,8 @@ IoctlEvtchnBindInterdomain(
>      if (!NT_SUCCESS(status))
>          goto fail3;
> 
> +    KeInitializeDpc(&Context->Dpc, EvtchnNotificationDpc, Context);
> +
>      status = STATUS_UNSUCCESSFUL;
>      Context->Channel = XENBUS_EVTCHN(Open,
>                                       &Fdo->EvtchnInterface,
> diff --git a/src/xeniface/ioctls.h b/src/xeniface/ioctls.h
> index 225ed7f..da273ce 100644
> --- a/src/xeniface/ioctls.h
> +++ b/src/xeniface/ioctls.h
> @@ -59,6 +59,7 @@ typedef struct _XENIFACE_EVTCHN_CONTEXT {
>      ULONG                  LocalPort;
>      PKEVENT                Event;
>      PXENIFACE_FDO          Fdo;
> +    KDPC                   Dpc;
>      PVOID                  FileObject;
>  } XENIFACE_EVTCHN_CONTEXT, *PXENIFACE_EVTCHN_CONTEXT;
> 
> --
> 2.1.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