From: "Fu, Siyuan" <siyuan.fu@intel.com>
To: "Wu, Jiaxin" <jiaxin.wu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Wang, Fan" <fan.wang@intel.com>, "Ye, Ting" <ting.ye@intel.com>
Subject: Re: [Patch] MdeModulePkg/Mtftp4Dxe: Separate the timer ticking to calculate the packet live time.
Date: Fri, 2 Mar 2018 06:45:52 +0000 [thread overview]
Message-ID: <B1FF2E9001CE9041BD10B825821D5BC58B449E02@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20180302064322.3600-1-jiaxin.wu@intel.com>
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
next prev parent reply other threads:[~2018-03-02 6:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2018-03-02 7:40 ` Ye, Ting
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B1FF2E9001CE9041BD10B825821D5BC58B449E02@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox