* [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.
@ 2018-03-02 6:43 Jiaxin Wu
2018-03-02 6:45 ` Fu, Siyuan
2018-03-02 7:40 ` Ye, Ting
0 siblings, 2 replies; 3+ messages in thread
From: Jiaxin Wu @ 2018-03-02 6:43 UTC (permalink / raw)
To: edk2-devel; +Cc: Fu Siyuan, Wang Fan, Ye Ting
From: Fu Siyuan <siyuan.fu@intel.com>
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 <fan.wang@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
.../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.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -160,15 +160,16 @@ Mtftp4CreateService (
MtftpSb->Signature = MTFTP4_SERVICE_SIGNATURE;
MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
MtftpSb->ChildrenNum = 0;
InitializeListHead (&MtftpSb->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,
&MtftpSb->Timer
);
+ if (EFI_ERROR (Status)) {
+ FreePool (MtftpSb);
+ return Status;
+ }
+ Status = gBS->CreateEvent (
+ EVT_NOTIFY_SIGNAL | EVT_TIMER,
+ TPL_NOTIFY,
+ Mtftp4OnTimerTickNotifyLevel,
+ MtftpSb,
+ &MtftpSb->TimerNotifyLevel
+ );
if (EFI_ERROR (Status)) {
+ gBS->CloseEvent (MtftpSb->Timer);
FreePool (MtftpSb);
return Status;
}
//
@@ -194,10 +207,11 @@ Mtftp4CreateService (
NULL,
NULL,
&MtftpSb->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 (
&Controller,
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_STATUS Status;
if (This == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -1094,11 +1095,13 @@ EfiMtftp4Poll (
} else if (Instance->State == MTFTP4_STATE_DESTROY) {
return EFI_DEVICE_ERROR;
}
Udp = Instance->UnicastPort->Protocol.Udp4;
- return Udp->Poll (Udp);
+ Status = Udp->Poll (Udp);
+ Mtftp4OnTimerTick (NULL, Instance->Service);
+ return Status;
}
EFI_MTFTP4_PROTOCOL gMtftp4ProtocolTemplate = {
EfiMtftp4GetModeData,
EfiMtftp4Configure,
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
index 527fd1db10..851b595eee 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
@@ -7,11 +7,11 @@
RFC2090 - TFTP Multicast Option
RFC2347 - TFTP Option Extension
RFC2348 - TFTP Blocksize Option
RFC2349 - TFTP Timeout Interval and Transfer Size Options
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -70,11 +70,12 @@ struct _MTFTP4_SERVICE {
EFI_SERVICE_BINDING_PROTOCOL ServiceBinding;
UINT16 ChildrenNum;
LIST_ENTRY Children;
- EFI_EVENT Timer; ///< Ticking timer for all the MTFTP clients
+ EFI_EVENT Timer; ///< Ticking timer for all the MTFTP clients to handle the packet timeout case.
+ EFI_EVENT TimerNotifyLevel; ///< Ticking timer for all the MTFTP clients to calculate the packet live time.
EFI_EVENT TimerToGetMap;
EFI_HANDLE Controller;
EFI_HANDLE Image;
@@ -133,10 +134,11 @@ struct _MTFTP4_PROTOCOL {
//
// Timeout and retransmit status
//
NET_BUF *LastPacket;
UINT32 PacketToLive;
+ BOOLEAN HasTimeout;
UINT32 CurRetry;
UINT32 MaxRetry;
UINT32 Timeout;
//
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index c625fda020..e4366b6ddb 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -1,9 +1,9 @@
/** @file
Support routines for Mtftp.
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -566,10 +566,46 @@ Mtftp4Retransmit (
return Status;
}
+/**
+ The timer ticking function in TPL_NOTIFY level for the Mtftp service instance.
+
+ @param Event The ticking event
+ @param Context The Mtftp service instance
+
+**/
+VOID
+EFIAPI
+Mtftp4OnTimerTickNotifyLevel (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ MTFTP4_SERVICE *MtftpSb;
+ LIST_ENTRY *Entry;
+ LIST_ENTRY *Next;
+ MTFTP4_PROTOCOL *Instance;
+
+ MtftpSb = (MTFTP4_SERVICE *) Context;
+
+ //
+ // Iterate through all the children of the Mtftp service instance. Time
+ // out the current packet transmit.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
+ Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
+ if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0)) {
+ Instance->HasTimeout = FALSE;
+ } else {
+ Instance->HasTimeout = TRUE;
+ }
+ }
+}
+
+
/**
The timer ticking function for the Mtftp service instance.
@param Event The ticking event
@param Context The Mtftp service instance
@@ -589,33 +625,32 @@ Mtftp4OnTimerTick (
EFI_MTFTP4_TOKEN *Token;
MtftpSb = (MTFTP4_SERVICE *) Context;
//
- // Iterate through all the children of the Mtftp service instance. Time
- // out the packet. If maximum retries reached, clean the session up.
+ // Iterate through all the children of the Mtftp service instance.
//
NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
-
- if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0)) {
+ if (!Instance->HasTimeout) {
continue;
}
+
+ Instance->HasTimeout = FALSE;
//
// Call the user's time out handler
//
Token = Instance->Token;
- if ((Token->TimeoutCallback != NULL) &&
+ if (Token != NULL && Token->TimeoutCallback != NULL &&
EFI_ERROR (Token->TimeoutCallback (&Instance->Mtftp4, Token))) {
-
Mtftp4SendError (
- Instance,
- EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
- (UINT8 *) "User aborted the transfer in time out"
- );
+ Instance,
+ EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
+ (UINT8 *) "User aborted the transfer in time out"
+ );
Mtftp4CleanOperation (Instance, EFI_ABORTED);
continue;
}
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
index 802e3867db..fd8703a925 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
@@ -1,9 +1,9 @@
/** @file
Support routines for MTFTP.
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -185,10 +185,24 @@ Mtftp4SendError (
EFI_STATUS
Mtftp4Retransmit (
IN MTFTP4_PROTOCOL *Instance
);
+/**
+ The timer ticking function in TPL_NOTIFY level for the Mtftp service instance.
+
+ @param Event The ticking event
+ @param Context The Mtftp service instance
+
+**/
+VOID
+EFIAPI
+Mtftp4OnTimerTickNotifyLevel (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ );
+
/**
The timer ticking function for the Mtftp service instance.
@param Event The ticking event
@param Context The Mtftp service instance
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.
2018-03-02 6:43 [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time Jiaxin Wu
@ 2018-03-02 6:45 ` Fu, Siyuan
2018-03-02 7:40 ` Ye, Ting
1 sibling, 0 replies; 3+ messages in thread
From: Fu, Siyuan @ 2018-03-02 6:45 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Wang, Fan, Ye, Ting
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, March 2, 2018 2:43 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wang, Fan <fan.wang@intel.com>; Ye,
> Ting <ting.ye@intel.com>
> Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to
> calculate the packet live time.
>
> From: Fu Siyuan <siyuan.fu@intel.com>
>
> 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 <fan.wang@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
> .../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.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 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<BR>
>
> @@ -160,15 +160,16 @@ Mtftp4CreateService (
> MtftpSb->Signature = MTFTP4_SERVICE_SIGNATURE;
> MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
> MtftpSb->ChildrenNum = 0;
> InitializeListHead (&MtftpSb->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,
> &MtftpSb->Timer
> );
> + if (EFI_ERROR (Status)) {
> + FreePool (MtftpSb);
> + return Status;
> + }
>
> + Status = gBS->CreateEvent (
> + EVT_NOTIFY_SIGNAL | EVT_TIMER,
> + TPL_NOTIFY,
> + Mtftp4OnTimerTickNotifyLevel,
> + MtftpSb,
> + &MtftpSb->TimerNotifyLevel
> + );
> if (EFI_ERROR (Status)) {
> + gBS->CloseEvent (MtftpSb->Timer);
> FreePool (MtftpSb);
> return Status;
> }
>
> //
> @@ -194,10 +207,11 @@ Mtftp4CreateService (
> NULL,
> NULL,
> &MtftpSb->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 (
> &Controller,
> 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_STATUS Status;
>
> if (This == NULL) {
> return EFI_INVALID_PARAMETER;
> }
>
> @@ -1094,11 +1095,13 @@ EfiMtftp4Poll (
> } else if (Instance->State == MTFTP4_STATE_DESTROY) {
> return EFI_DEVICE_ERROR;
> }
>
> Udp = Instance->UnicastPort->Protocol.Udp4;
> - return Udp->Poll (Udp);
> + Status = Udp->Poll (Udp);
> + Mtftp4OnTimerTick (NULL, Instance->Service);
> + return Status;
> }
>
> EFI_MTFTP4_PROTOCOL gMtftp4ProtocolTemplate = {
> EfiMtftp4GetModeData,
> EfiMtftp4Configure,
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
> index 527fd1db10..851b595eee 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
> @@ -7,11 +7,11 @@
> RFC2090 - TFTP Multicast Option
> RFC2347 - TFTP Option Extension
> RFC2348 - TFTP Blocksize Option
> RFC2349 - TFTP Timeout Interval and Transfer Size Options
>
> -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 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<BR>
>
> @@ -70,11 +70,12 @@ struct _MTFTP4_SERVICE {
> EFI_SERVICE_BINDING_PROTOCOL ServiceBinding;
>
> UINT16 ChildrenNum;
> LIST_ENTRY Children;
>
> - EFI_EVENT Timer; ///< Ticking timer for all the
> MTFTP clients
> + EFI_EVENT Timer; ///< Ticking timer for all the
> MTFTP clients to handle the packet timeout case.
> + EFI_EVENT TimerNotifyLevel; ///< Ticking timer for
> all the MTFTP clients to calculate the packet live time.
> EFI_EVENT TimerToGetMap;
>
> EFI_HANDLE Controller;
> EFI_HANDLE Image;
>
> @@ -133,10 +134,11 @@ struct _MTFTP4_PROTOCOL {
> //
> // Timeout and retransmit status
> //
> NET_BUF *LastPacket;
> UINT32 PacketToLive;
> + BOOLEAN HasTimeout;
> UINT32 CurRetry;
> UINT32 MaxRetry;
> UINT32 Timeout;
>
> //
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
> index c625fda020..e4366b6ddb 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
> @@ -1,9 +1,9 @@
> /** @file
> Support routines for Mtftp.
>
> -Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 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<BR>
>
> @@ -566,10 +566,46 @@ Mtftp4Retransmit (
>
> return Status;
> }
>
>
> +/**
> + The timer ticking function in TPL_NOTIFY level for the Mtftp service
> instance.
> +
> + @param Event The ticking event
> + @param Context The Mtftp service instance
> +
> +**/
> +VOID
> +EFIAPI
> +Mtftp4OnTimerTickNotifyLevel (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + MTFTP4_SERVICE *MtftpSb;
> + LIST_ENTRY *Entry;
> + LIST_ENTRY *Next;
> + MTFTP4_PROTOCOL *Instance;
> +
> + MtftpSb = (MTFTP4_SERVICE *) Context;
> +
> + //
> + // Iterate through all the children of the Mtftp service instance. Time
> + // out the current packet transmit.
> + //
> + NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
> + Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
> + if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0))
> {
> + Instance->HasTimeout = FALSE;
> + } else {
> + Instance->HasTimeout = TRUE;
> + }
> + }
> +}
> +
> +
> /**
> The timer ticking function for the Mtftp service instance.
>
> @param Event The ticking event
> @param Context The Mtftp service instance
> @@ -589,33 +625,32 @@ Mtftp4OnTimerTick (
> EFI_MTFTP4_TOKEN *Token;
>
> MtftpSb = (MTFTP4_SERVICE *) Context;
>
> //
> - // Iterate through all the children of the Mtftp service instance. Time
> - // out the packet. If maximum retries reached, clean the session up.
> + // Iterate through all the children of the Mtftp service instance.
> //
> NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
> Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
> -
> - if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0))
> {
> + if (!Instance->HasTimeout) {
> continue;
> }
> +
> + Instance->HasTimeout = FALSE;
>
> //
> // Call the user's time out handler
> //
> Token = Instance->Token;
>
> - if ((Token->TimeoutCallback != NULL) &&
> + if (Token != NULL && Token->TimeoutCallback != NULL &&
> EFI_ERROR (Token->TimeoutCallback (&Instance->Mtftp4, Token))) {
> -
> Mtftp4SendError (
> - Instance,
> - EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
> - (UINT8 *) "User aborted the transfer in time out"
> - );
> + Instance,
> + EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
> + (UINT8 *) "User aborted the transfer in time out"
> + );
>
> Mtftp4CleanOperation (Instance, EFI_ABORTED);
> continue;
> }
>
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
> index 802e3867db..fd8703a925 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
> @@ -1,9 +1,9 @@
> /** @file
> Support routines for MTFTP.
>
> -Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> 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<BR>
>
> @@ -185,10 +185,24 @@ Mtftp4SendError (
> EFI_STATUS
> Mtftp4Retransmit (
> IN MTFTP4_PROTOCOL *Instance
> );
>
> +/**
> + The timer ticking function in TPL_NOTIFY level for the Mtftp service
> instance.
> +
> + @param Event The ticking event
> + @param Context The Mtftp service instance
> +
> +**/
> +VOID
> +EFIAPI
> +Mtftp4OnTimerTickNotifyLevel (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + );
> +
> /**
> The timer ticking function for the Mtftp service instance.
>
> @param Event The ticking event
> @param Context The Mtftp service instance
> --
> 2.16.2.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.
2018-03-02 6:43 [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time Jiaxin Wu
2018-03-02 6:45 ` Fu, Siyuan
@ 2018-03-02 7:40 ` Ye, Ting
1 sibling, 0 replies; 3+ messages in thread
From: Ye, Ting @ 2018-03-02 7:40 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Wang, Fan
Reviewed-by: Ye Ting <ting.ye@intel.com>
-----Original Message-----
From: Wu, Jiaxin
Sent: Friday, March 2, 2018 2:43 PM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan <siyuan.fu@intel.com>; Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.
From: Fu Siyuan <siyuan.fu@intel.com>
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 <fan.wang@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
---
.../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.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -160,15 +160,16 @@ Mtftp4CreateService (
MtftpSb->Signature = MTFTP4_SERVICE_SIGNATURE;
MtftpSb->ServiceBinding = gMtftp4ServiceBindingTemplete;
MtftpSb->ChildrenNum = 0;
InitializeListHead (&MtftpSb->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,
&MtftpSb->Timer
);
+ if (EFI_ERROR (Status)) {
+ FreePool (MtftpSb);
+ return Status;
+ }
+ Status = gBS->CreateEvent (
+ EVT_NOTIFY_SIGNAL | EVT_TIMER,
+ TPL_NOTIFY,
+ Mtftp4OnTimerTickNotifyLevel,
+ MtftpSb,
+ &MtftpSb->TimerNotifyLevel
+ );
if (EFI_ERROR (Status)) {
+ gBS->CloseEvent (MtftpSb->Timer);
FreePool (MtftpSb);
return Status;
}
//
@@ -194,10 +207,11 @@ Mtftp4CreateService (
NULL,
NULL,
&MtftpSb->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 (
&Controller,
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_STATUS Status;
if (This == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -1094,11 +1095,13 @@ EfiMtftp4Poll (
} else if (Instance->State == MTFTP4_STATE_DESTROY) {
return EFI_DEVICE_ERROR;
}
Udp = Instance->UnicastPort->Protocol.Udp4;
- return Udp->Poll (Udp);
+ Status = Udp->Poll (Udp);
+ Mtftp4OnTimerTick (NULL, Instance->Service); return Status;
}
EFI_MTFTP4_PROTOCOL gMtftp4ProtocolTemplate = {
EfiMtftp4GetModeData,
EfiMtftp4Configure,
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
index 527fd1db10..851b595eee 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h
@@ -7,11 +7,11 @@
RFC2090 - TFTP Multicast Option
RFC2347 - TFTP Option Extension
RFC2348 - TFTP Blocksize Option
RFC2349 - TFTP Timeout Interval and Transfer Size Options
-Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -70,11 +70,12 @@ struct _MTFTP4_SERVICE {
EFI_SERVICE_BINDING_PROTOCOL ServiceBinding;
UINT16 ChildrenNum;
LIST_ENTRY Children;
- EFI_EVENT Timer; ///< Ticking timer for all the MTFTP clients
+ EFI_EVENT Timer; ///< Ticking timer for all the MTFTP clients to handle the packet timeout case.
+ EFI_EVENT TimerNotifyLevel; ///< Ticking timer for all the MTFTP clients to calculate the packet live time.
EFI_EVENT TimerToGetMap;
EFI_HANDLE Controller;
EFI_HANDLE Image;
@@ -133,10 +134,11 @@ struct _MTFTP4_PROTOCOL {
//
// Timeout and retransmit status
//
NET_BUF *LastPacket;
UINT32 PacketToLive;
+ BOOLEAN HasTimeout;
UINT32 CurRetry;
UINT32 MaxRetry;
UINT32 Timeout;
//
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
index c625fda020..e4366b6ddb 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c
@@ -1,9 +1,9 @@
/** @file
Support routines for Mtftp.
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -566,10 +566,46 @@ Mtftp4Retransmit (
return Status;
}
+/**
+ The timer ticking function in TPL_NOTIFY level for the Mtftp service instance.
+
+ @param Event The ticking event
+ @param Context The Mtftp service instance
+
+**/
+VOID
+EFIAPI
+Mtftp4OnTimerTickNotifyLevel (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ MTFTP4_SERVICE *MtftpSb;
+ LIST_ENTRY *Entry;
+ LIST_ENTRY *Next;
+ MTFTP4_PROTOCOL *Instance;
+
+ MtftpSb = (MTFTP4_SERVICE *) Context;
+
+ //
+ // Iterate through all the children of the Mtftp service instance. Time
+ // out the current packet transmit.
+ //
+ NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
+ Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
+ if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0)) {
+ Instance->HasTimeout = FALSE;
+ } else {
+ Instance->HasTimeout = TRUE;
+ }
+ }
+}
+
+
/**
The timer ticking function for the Mtftp service instance.
@param Event The ticking event
@param Context The Mtftp service instance
@@ -589,33 +625,32 @@ Mtftp4OnTimerTick (
EFI_MTFTP4_TOKEN *Token;
MtftpSb = (MTFTP4_SERVICE *) Context;
//
- // Iterate through all the children of the Mtftp service instance. Time
- // out the packet. If maximum retries reached, clean the session up.
+ // Iterate through all the children of the Mtftp service instance.
//
NET_LIST_FOR_EACH_SAFE (Entry, Next, &MtftpSb->Children) {
Instance = NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link);
-
- if ((Instance->PacketToLive == 0) || (--Instance->PacketToLive > 0)) {
+ if (!Instance->HasTimeout) {
continue;
}
+
+ Instance->HasTimeout = FALSE;
//
// Call the user's time out handler
//
Token = Instance->Token;
- if ((Token->TimeoutCallback != NULL) &&
+ if (Token != NULL && Token->TimeoutCallback != NULL &&
EFI_ERROR (Token->TimeoutCallback (&Instance->Mtftp4, Token))) {
-
Mtftp4SendError (
- Instance,
- EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
- (UINT8 *) "User aborted the transfer in time out"
- );
+ Instance,
+ EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
+ (UINT8 *) "User aborted the transfer in time out"
+ );
Mtftp4CleanOperation (Instance, EFI_ABORTED);
continue;
}
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
index 802e3867db..fd8703a925 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h
@@ -1,9 +1,9 @@
/** @file
Support routines for MTFTP.
-Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
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<BR>
@@ -185,10 +185,24 @@ Mtftp4SendError (
EFI_STATUS
Mtftp4Retransmit (
IN MTFTP4_PROTOCOL *Instance
);
+/**
+ The timer ticking function in TPL_NOTIFY level for the Mtftp service instance.
+
+ @param Event The ticking event
+ @param Context The Mtftp service instance
+
+**/
+VOID
+EFIAPI
+Mtftp4OnTimerTickNotifyLevel (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ );
+
/**
The timer ticking function for the Mtftp service instance.
@param Event The ticking event
@param Context The Mtftp service instance
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-02 7:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-02 6:43 [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time Jiaxin Wu
2018-03-02 6:45 ` Fu, Siyuan
2018-03-02 7:40 ` Ye, Ting
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox