From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=ting.ye@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 16546223C1790 for ; Thu, 1 Mar 2018 23:34:48 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Mar 2018 23:40:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,411,1515484800"; d="scan'208";a="34916488" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 01 Mar 2018 23:40:57 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 1 Mar 2018 23:40:57 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.116]) by shsmsx102.ccr.corp.intel.com ([169.254.2.124]) with mapi id 14.03.0319.002; Fri, 2 Mar 2018 15:40:55 +0800 From: "Ye, Ting" To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" CC: "Fu, Siyuan" , "Wang, Fan" Thread-Topic: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time. Thread-Index: AQHTsfHHKqkTH+YzSEqBPzuPWCg+yaO8j5yg Date: Fri, 2 Mar 2018 07:40:54 +0000 Message-ID: References: <20180302064322.3600-1-jiaxin.wu@intel.com> In-Reply-To: <20180302064322.3600-1-jiaxin.wu@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Mar 2018 07:34:49 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ye Ting =20 -----Original Message----- From: Wu, Jiaxin=20 Sent: Friday, March 2, 2018 2:43 PM To: edk2-devel@lists.01.org Cc: Fu, Siyuan ; Wang, Fan ; Ye, T= ing Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calc= ulate the packet live time. From: Fu Siyuan TPL deadlock issue was enrolled by the commit of 39b0867d. To resolve the i= ssue, this patch separated the timer ticking for all the MTFTP clients to c= alculate 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/MdeM= odulePkg/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. =20 -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 availab= le under the terms and conditions of the BSD License which accompanies thi= s distribution. The full text of the license may be found at http://opens= ource.org/licenses/bsd-license.php
=20 @@ -160,15 +160,16 @@ Mtftp4CreateService ( MtftpSb->Signature =3D MTFTP4_SERVICE_SIGNATURE; MtftpSb->ServiceBinding =3D gMtftp4ServiceBindingTemplete; MtftpSb->ChildrenNum =3D 0; InitializeListHead (&MtftpSb->Children); =20 - MtftpSb->Timer =3D NULL; - MtftpSb->TimerToGetMap =3D NULL; - MtftpSb->Controller =3D Controller; - MtftpSb->Image =3D Image; - MtftpSb->ConnectUdp =3D NULL; + MtftpSb->Timer =3D NULL; + MtftpSb->TimerNotifyLevel =3D NULL; + MtftpSb->TimerToGetMap =3D NULL; + MtftpSb->Controller =3D Controller; + MtftpSb->Image =3D Image; + MtftpSb->ConnectUdp =3D NULL; =20 // // Create the timer and a udp to be notified when UDP is uninstalled // Status =3D gBS->CreateEvent ( @@ -176,12 +177,24 @@ Mtftp4CreateService ( TPL_CALLBACK, Mtftp4OnTimerTick, MtftpSb, &MtftpSb->Timer ); + if (EFI_ERROR (Status)) { + FreePool (MtftpSb); + return Status; + } =20 + Status =3D 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; } =20 // @@ -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; } =20 @@ -209,10 +223,11 @@ Mtftp4CreateService ( NULL ); =20 if (MtftpSb->ConnectUdp =3D=3D NULL) { gBS->CloseEvent (MtftpSb->TimerToGetMap); + gBS->CloseEvent (MtftpSb->TimerNotifyLevel); gBS->CloseEvent (MtftpSb->Timer); FreePool (MtftpSb); return EFI_DEVICE_ERROR; } =20 @@ -232,10 +247,11 @@ Mtftp4CleanService ( IN MTFTP4_SERVICE *MtftpSb ) { UdpIoFreeIo (MtftpSb->ConnectUdp); gBS->CloseEvent (MtftpSb->TimerToGetMap); + gBS->CloseEvent (MtftpSb->TimerNotifyLevel); gBS->CloseEvent (MtftpSb->Timer); } =20 =20 /** @@ -292,10 +308,16 @@ Mtftp4DriverBindingStart ( =20 if (EFI_ERROR (Status)) { goto ON_ERROR; } =20 + Status =3D gBS->SetTimer (MtftpSb->TimerNotifyLevel, TimerPeriodic,=20 + TICKS_PER_SECOND); + + if (EFI_ERROR (Status)) { + goto ON_ERROR; + } + =20 // // Install the Mtftp4ServiceBinding Protocol onto Controller // Status =3D gBS->InstallMultipleProtocolInterfaces ( &Controller, diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c b/MdeMod= ulePkg/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; =20 if (This =3D=3D NULL) { return EFI_INVALID_PARAMETER; } =20 @@ -1094,11 +1095,13 @@ EfiMtftp4Poll ( } else if (Instance->State =3D=3D MTFTP4_STATE_DESTROY) { return EFI_DEVICE_ERROR; } =20 Udp =3D Instance->UnicastPort->Protocol.Udp4; - return Udp->Poll (Udp); + Status =3D Udp->Poll (Udp); + Mtftp4OnTimerTick (NULL, Instance->Service); return Status; } =20 EFI_MTFTP4_PROTOCOL gMtftp4ProtocolTemplate =3D { EfiMtftp4GetModeData, EfiMtftp4Configure, diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeMod= ulePkg/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 =20 -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
=20 @@ -70,11 +70,12 @@ struct _MTFTP4_SERVICE { EFI_SERVICE_BINDING_PROTOCOL ServiceBinding; =20 UINT16 ChildrenNum; LIST_ENTRY Children; =20 - EFI_EVENT Timer; ///< Ticking timer for all the MTF= TP clients + EFI_EVENT Timer; ///< Ticking timer for all the MTF= TP clients to handle the packet timeout case. + EFI_EVENT TimerNotifyLevel; ///< Ticking timer for a= ll the MTFTP clients to calculate the packet live time. EFI_EVENT TimerToGetMap; =20 EFI_HANDLE Controller; EFI_HANDLE Image; =20 @@ -133,10 +134,11 @@ struct _MTFTP4_PROTOCOL { // // Timeout and retransmit status // NET_BUF *LastPacket; UINT32 PacketToLive; + BOOLEAN HasTimeout; UINT32 CurRetry; UINT32 MaxRetry; UINT32 Timeout; =20 // diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/Mde= ModulePkg/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. =20 -Copyright (c) 2006 - 2015, 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
=20 @@ -566,10 +566,46 @@ Mtftp4Retransmit ( =20 return Status; } =20 =20 +/** + The timer ticking function in TPL_NOTIFY level for the Mtftp service ins= tance. + + @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 =3D (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 =3D NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link); + if ((Instance->PacketToLive =3D=3D 0) || (--Instance->PacketToLive > 0= )) { + Instance->HasTimeout =3D FALSE; + } else { + Instance->HasTimeout =3D TRUE; + } + } +} + + /** The timer ticking function for the Mtftp service instance. =20 @param Event The ticking event @param Context The Mtftp service instance @@ -589,33 +625,32 @@ Mtftp4OnTimerTick ( EFI_MTFTP4_TOKEN *Token; =20 MtftpSb =3D (MTFTP4_SERVICE *) Context; =20 // - // 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 =3D NET_LIST_USER_STRUCT (Entry, MTFTP4_PROTOCOL, Link); - - if ((Instance->PacketToLive =3D=3D 0) || (--Instance->PacketToLive > 0= )) { + if (!Instance->HasTimeout) { continue; } + =20 + Instance->HasTimeout =3D FALSE; =20 // // Call the user's time out handler // Token =3D Instance->Token; =20 - if ((Token->TimeoutCallback !=3D NULL) && + if (Token !=3D NULL && Token->TimeoutCallback !=3D 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" + ); =20 Mtftp4CleanOperation (Instance, EFI_ABORTED); continue; } =20 diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/Mde= ModulePkg/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. =20 -Copyright (c) 2006 - 2010, 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
=20 @@ -185,10 +185,24 @@ Mtftp4SendError ( EFI_STATUS Mtftp4Retransmit ( IN MTFTP4_PROTOCOL *Instance ); =20 +/** + The timer ticking function in TPL_NOTIFY level for the Mtftp service ins= tance. + + @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. =20 @param Event The ticking event @param Context The Mtftp service instance --=20 2.16.2.windows.1