Re: [edk2] [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.

2018-03-01 Thread Ye, Ting 
Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Friday, March 2, 2018 2:43 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Wang, Fan ; Ye, Ting 

Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to 
calculate the packet live time.

From: Fu Siyuan 

TPL deadlock issue was enrolled by the commit of 39b0867d. To resolve the 
issue, this patch separated the timer ticking for all the MTFTP clients to 
calculate the packet live time in TPL_NOTIFY level.

Cc: Wang Fan 
Cc: Fu Siyuan 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Signed-off-by: Jiaxin Wu 
---
 .../Universal/Network/Mtftp4Dxe/Mtftp4Driver.c | 34 ++---
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c   |  5 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h   |  6 ++-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Support.c| 57 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Support.h| 16 +-
 5 files changed, 97 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
index a23d405baa..713cc66dd1 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of Mtftp drivers.
 
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at  
http://opensource.org/licenses/bsd-license.php
 
@@ -160,15 +160,16 @@ Mtftp4CreateService (
   MtftpSb->Signature  = MTFTP4_SERVICE_SIGNATURE;
   MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
   MtftpSb->ChildrenNum= 0;
   InitializeListHead (>Children);
 
-  MtftpSb->Timer  = NULL;
-  MtftpSb->TimerToGetMap  = NULL;
-  MtftpSb->Controller = Controller;
-  MtftpSb->Image  = Image;
-  MtftpSb->ConnectUdp = NULL;
+  MtftpSb->Timer= NULL;
+  MtftpSb->TimerNotifyLevel = NULL;
+  MtftpSb->TimerToGetMap= NULL;
+  MtftpSb->Controller   = Controller;
+  MtftpSb->Image= Image;
+  MtftpSb->ConnectUdp   = NULL;
 
   //
   // Create the timer and a udp to be notified when UDP is uninstalled
   //
   Status = gBS->CreateEvent (
@@ -176,12 +177,24 @@ Mtftp4CreateService (
   TPL_CALLBACK,
   Mtftp4OnTimerTick,
   MtftpSb,
   >Timer
   );
+  if (EFI_ERROR (Status)) {
+FreePool (MtftpSb);
+return Status;
+  }
 
+  Status = gBS->CreateEvent (
+  EVT_NOTIFY_SIGNAL | EVT_TIMER,
+  TPL_NOTIFY,
+  Mtftp4OnTimerTickNotifyLevel,
+  MtftpSb,
+  >TimerNotifyLevel
+  );
   if (EFI_ERROR (Status)) {
+gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return Status;
   }
 
   //
@@ -194,10 +207,11 @@ Mtftp4CreateService (
   NULL,
   NULL,
   >TimerToGetMap
   );
   if (EFI_ERROR (Status)) {
+gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
 gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return Status;
   }
 
@@ -209,10 +223,11 @@ Mtftp4CreateService (
   NULL
   );
 
   if (MtftpSb->ConnectUdp == NULL) {
 gBS->CloseEvent (MtftpSb->TimerToGetMap);
+gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
 gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return EFI_DEVICE_ERROR;
   }
 
@@ -232,10 +247,11 @@ Mtftp4CleanService (
   IN MTFTP4_SERVICE *MtftpSb
   )
 {
   UdpIoFreeIo (MtftpSb->ConnectUdp);
   gBS->CloseEvent (MtftpSb->TimerToGetMap);
+  gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
   gBS->CloseEvent (MtftpSb->Timer);
 }
 
 
 /**
@@ -292,10 +308,16 @@ Mtftp4DriverBindingStart (
 
   if (EFI_ERROR (Status)) {
 goto ON_ERROR;
   }
 
+  Status = gBS->SetTimer (MtftpSb->TimerNotifyLevel, TimerPeriodic, 
+ TICKS_PER_SECOND);
+
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
+  
   //
   // Install the Mtftp4ServiceBinding Protocol onto Controller
   //
   Status = gBS->InstallMultipleProtocolInterfaces (
   ,
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index f5f9e6d8f7..d8c48ec8b2 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++

Re: [edk2] [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.

2018-03-01 Thread Fu, Siyuan 
Reviewed-by: Fu Siyuan 


> -Original Message-
> From: Wu, Jiaxin
> Sent: Friday, March 2, 2018 2:43 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Wang, Fan ; Ye,
> Ting 
> Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to
> calculate the packet live time.
> 
> From: Fu Siyuan 
> 
> TPL deadlock issue was enrolled by the commit of 39b0867d. To resolve the
> issue,
> this patch separated the timer ticking for all the MTFTP clients to
> calculate the
> packet live time in TPL_NOTIFY level.
> 
> Cc: Wang Fan 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Signed-off-by: Jiaxin Wu 
> ---
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Driver.c | 34 ++---
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c   |  5 +-
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h   |  6 ++-
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Support.c| 57
> +-
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Support.h| 16 +-
>  5 files changed, 97 insertions(+), 21 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
> index a23d405baa..713cc66dd1 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
> @@ -1,9 +1,9 @@
>  /** @file
>Implementation of Mtftp drivers.
> 
> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -160,15 +160,16 @@ Mtftp4CreateService (
>MtftpSb->Signature  = MTFTP4_SERVICE_SIGNATURE;
>MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
>MtftpSb->ChildrenNum= 0;
>InitializeListHead (>Children);
> 
> -  MtftpSb->Timer  = NULL;
> -  MtftpSb->TimerToGetMap  = NULL;
> -  MtftpSb->Controller = Controller;
> -  MtftpSb->Image  = Image;
> -  MtftpSb->ConnectUdp = NULL;
> +  MtftpSb->Timer= NULL;
> +  MtftpSb->TimerNotifyLevel = NULL;
> +  MtftpSb->TimerToGetMap= NULL;
> +  MtftpSb->Controller   = Controller;
> +  MtftpSb->Image= Image;
> +  MtftpSb->ConnectUdp   = NULL;
> 
>//
>// Create the timer and a udp to be notified when UDP is uninstalled
>//
>Status = gBS->CreateEvent (
> @@ -176,12 +177,24 @@ Mtftp4CreateService (
>TPL_CALLBACK,
>Mtftp4OnTimerTick,
>MtftpSb,
>>Timer
>);
> +  if (EFI_ERROR (Status)) {
> +FreePool (MtftpSb);
> +return Status;
> +  }
> 
> +  Status = gBS->CreateEvent (
> +  EVT_NOTIFY_SIGNAL | EVT_TIMER,
> +  TPL_NOTIFY,
> +  Mtftp4OnTimerTickNotifyLevel,
> +  MtftpSb,
> +  >TimerNotifyLevel
> +  );
>if (EFI_ERROR (Status)) {
> +gBS->CloseEvent (MtftpSb->Timer);
>  FreePool (MtftpSb);
>  return Status;
>}
> 
>//
> @@ -194,10 +207,11 @@ Mtftp4CreateService (
>NULL,
>NULL,
>>TimerToGetMap
>);
>if (EFI_ERROR (Status)) {
> +gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
>  gBS->CloseEvent (MtftpSb->Timer);
>  FreePool (MtftpSb);
>  return Status;
>}
> 
> @@ -209,10 +223,11 @@ Mtftp4CreateService (
>NULL
>);
> 
>if (MtftpSb->ConnectUdp == NULL) {
>  gBS->CloseEvent (MtftpSb->TimerToGetMap);
> +gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
>  gBS->CloseEvent (MtftpSb->Timer);
>  FreePool (MtftpSb);
>  return EFI_DEVICE_ERROR;
>}
> 
> @@ -232,10 +247,11 @@ Mtftp4CleanService (
>IN MTFTP4_SERVICE *MtftpSb
>)
>  {
>UdpIoFreeIo (MtftpSb->ConnectUdp);
>gBS->CloseEvent (MtftpSb->TimerToGetMap);
> +  gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
>gBS->CloseEvent (MtftpSb->Timer);
>  }
> 
> 
>  /**
> @@ -292,10 +308,16 @@ Mtftp4DriverBindingStart (
> 
>if (EFI_ERROR (Status)) {
>  goto ON_ERROR;
>}
> 
> +  Status = gBS->SetTimer (MtftpSb->TimerNotifyLevel, TimerPeriodic,
> TICKS_PER_SECOND);
> +
> +  if (EFI_ERROR (Status)) {
> +goto ON_ERROR;
> +  }
> +
>//
>// Install the Mtftp4ServiceBinding Protocol onto Controller
>//
>Status = gBS->InstallMultipleProtocolInterfaces (
>

[edk2] [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.

2018-03-01 Thread Jiaxin Wu 
From: Fu Siyuan 

TPL deadlock issue was enrolled by the commit of 39b0867d. To resolve the issue,
this patch separated the timer ticking for all the MTFTP clients to calculate 
the
packet live time in TPL_NOTIFY level.

Cc: Wang Fan 
Cc: Fu Siyuan 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
Signed-off-by: Jiaxin Wu 
---
 .../Universal/Network/Mtftp4Dxe/Mtftp4Driver.c | 34 ++---
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c   |  5 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h   |  6 ++-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Support.c| 57 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Support.h| 16 +-
 5 files changed, 97 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
index a23d405baa..713cc66dd1 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Driver.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of Mtftp drivers.
 
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -160,15 +160,16 @@ Mtftp4CreateService (
   MtftpSb->Signature  = MTFTP4_SERVICE_SIGNATURE;
   MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
   MtftpSb->ChildrenNum= 0;
   InitializeListHead (>Children);
 
-  MtftpSb->Timer  = NULL;
-  MtftpSb->TimerToGetMap  = NULL;
-  MtftpSb->Controller = Controller;
-  MtftpSb->Image  = Image;
-  MtftpSb->ConnectUdp = NULL;
+  MtftpSb->Timer= NULL;
+  MtftpSb->TimerNotifyLevel = NULL;
+  MtftpSb->TimerToGetMap= NULL;
+  MtftpSb->Controller   = Controller;
+  MtftpSb->Image= Image;
+  MtftpSb->ConnectUdp   = NULL;
 
   //
   // Create the timer and a udp to be notified when UDP is uninstalled
   //
   Status = gBS->CreateEvent (
@@ -176,12 +177,24 @@ Mtftp4CreateService (
   TPL_CALLBACK,
   Mtftp4OnTimerTick,
   MtftpSb,
   >Timer
   );
+  if (EFI_ERROR (Status)) {
+FreePool (MtftpSb);
+return Status;
+  }
 
+  Status = gBS->CreateEvent (
+  EVT_NOTIFY_SIGNAL | EVT_TIMER,
+  TPL_NOTIFY,
+  Mtftp4OnTimerTickNotifyLevel,
+  MtftpSb,
+  >TimerNotifyLevel
+  );
   if (EFI_ERROR (Status)) {
+gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return Status;
   }
 
   //
@@ -194,10 +207,11 @@ Mtftp4CreateService (
   NULL,
   NULL,
   >TimerToGetMap
   );
   if (EFI_ERROR (Status)) {
+gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
 gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return Status;
   }
 
@@ -209,10 +223,11 @@ Mtftp4CreateService (
   NULL
   );
 
   if (MtftpSb->ConnectUdp == NULL) {
 gBS->CloseEvent (MtftpSb->TimerToGetMap);
+gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
 gBS->CloseEvent (MtftpSb->Timer);
 FreePool (MtftpSb);
 return EFI_DEVICE_ERROR;
   }
 
@@ -232,10 +247,11 @@ Mtftp4CleanService (
   IN MTFTP4_SERVICE *MtftpSb
   )
 {
   UdpIoFreeIo (MtftpSb->ConnectUdp);
   gBS->CloseEvent (MtftpSb->TimerToGetMap);
+  gBS->CloseEvent (MtftpSb->TimerNotifyLevel);
   gBS->CloseEvent (MtftpSb->Timer);
 }
 
 
 /**
@@ -292,10 +308,16 @@ Mtftp4DriverBindingStart (
 
   if (EFI_ERROR (Status)) {
 goto ON_ERROR;
   }
 
+  Status = gBS->SetTimer (MtftpSb->TimerNotifyLevel, TimerPeriodic, 
TICKS_PER_SECOND);
+
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
+  
   //
   // Install the Mtftp4ServiceBinding Protocol onto Controller
   //
   Status = gBS->InstallMultipleProtocolInterfaces (
   ,
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index f5f9e6d8f7..d8c48ec8b2 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
@@ -1080,10 +1080,11 @@ EfiMtftp4Poll (
   IN EFI_MTFTP4_PROTOCOL*This
   )
 {
   MTFTP4_PROTOCOL   *Instance;
   EFI_UDP4_PROTOCOL *Udp;
+  EFI_STATUSStatus;
 
   if (This == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -1094,11 +1095,13 @@ EfiMtftp4Poll (
   } else if (Instance->State ==