Re: [PATCH-V1] MdeModulePkg/NvmExpressDxe: Memory leak fix in async code flow

["Wu, Hao A" <hao.a.wu@intel.com>]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>
I have removed some tab usages and trailing white space cases in the patch.

Pushed as commit: f2333c707dd04f069c102bcae004561ec590a3a0

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Suman Prakash
> Sent: Monday, March 20, 2017 4:35 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng
> Subject: [edk2] [PATCH-V1] MdeModulePkg/NvmExpressDxe: Memory leak
> fix in async code flow
> 
>     MdeModulePkg/NvmExpressDxe: Memory leak fix
>     in async code flow
> 
>     For async commands, the buffer allocated for Prp list is
>     not getting freed, which will cause memory leak for async
>     read write command. For example testing async command flow
>     with custom application to send multiple read write commands
>     were resulting in decrease of available memory page in memmap,
>     which eventually resulted in system hang. Hence freeing AsyncRequest-
> >MapData,
>     AsyncRequest->MapMeta, AsyncRequest->MapPrpList and AsyncRequest-
> >PrpListHost
>     when async command is completed.
> 
>     Cc: Feng Tian <feng.tian@intel.com>
>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Suman Prakash <suman.p@samsung.com.com>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c    | 23
> +++++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h    |  5 +++++
>  .../Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c     |  5 +++++
>  3 files changed, 32 insertions(+), 1 deletion(-)
>  mode change 100644 => 100755
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
>  mode change 100644 => 100755
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
>  mode change 100644 => 100755
> MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> old mode 100644
> new mode 100755
> index 39f49bd..ec6af58
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> @@ -548,6 +548,8 @@ ProcessAsyncTaskList (
>    QueueId    = 2;
>    Cq         = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
>    HasNewItem = FALSE;
> +  PciIo		= Private->PciIo;
> +
> 
>    //
>    // Submit asynchronous subtasks to the NVMe Submission Queue
> @@ -644,6 +646,26 @@ ProcessAsyncTaskList (
>            sizeof(EFI_NVM_EXPRESS_COMPLETION)
>            );
> 
> +	//
> +	// Free the resources allocated before cmd submission
> +	//
> +	if (AsyncRequest->MapData != NULL) {
> +		PciIo->Unmap (PciIo, AsyncRequest->MapData);
> +	}
> +	if (AsyncRequest->MapMeta != NULL) {
> +		PciIo->Unmap (PciIo, AsyncRequest->MapMeta);
> +	}
> +	if (AsyncRequest->MapPrpList != NULL) {
> +		PciIo->Unmap (PciIo, AsyncRequest->MapPrpList);
> +	}
> +	if (AsyncRequest->PrpListHost != NULL) {
> +		PciIo->FreeBuffer (
> +				PciIo,
> +				AsyncRequest->PrpListNo,
> +				AsyncRequest->PrpListHost
> +				);
> +	}
> +
>          RemoveEntryList (Link);
>          gBS->SignalEvent (AsyncRequest->CallerEvent);
>          FreePool (AsyncRequest);
> @@ -666,7 +688,6 @@ ProcessAsyncTaskList (
>    }
> 
>    if (HasNewItem) {
> -    PciIo = Private->PciIo;
>      Data  = ReadUnaligned32 ((UINT32*)&Private->CqHdbl[QueueId]);
>      PciIo->Mem.Write (
>                   PciIo,
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
> old mode 100644
> new mode 100755
> index 6a1c257..fa4a34a
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.h
> @@ -292,6 +292,11 @@ typedef struct {
> 
>    EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET *Packet;
>    UINT16                                   CommandId;
> +  VOID                                     *MapPrpList;
> +  UINTN                                    PrpListNo;
> +  VOID                                     *PrpListHost;
> +  VOID                                     *MapData;
> +  VOID                                     *MapMeta;
>    EFI_EVENT                                CallerEvent;
>  } NVME_PASS_THRU_ASYNC_REQ;
> 
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> old mode 100644
> new mode 100755
> index 2c30009..f0cca72
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -627,6 +627,11 @@ NvmExpressPassThru (
>      AsyncRequest->Packet        = Packet;
>      AsyncRequest->CommandId     = Sq->Cid;
>      AsyncRequest->CallerEvent   = Event;
> +    AsyncRequest->MapData	= MapData;
> +    AsyncRequest->MapMeta	= MapMeta;
> +    AsyncRequest->MapPrpList	= MapPrpList;
> +    AsyncRequest->PrpListNo	= PrpListNo;
> +    AsyncRequest->PrpListHost	= PrpListHost;
> 
>      OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>      InsertTailList (&Private->AsyncPassThruQueue, &AsyncRequest->Link);
> --
> 1.9.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel