* Re: [PATCH-V1] MdeModulePkg/NvmExpressDxe: Memory leak fix in async code flow
2017-03-20 8:34 ` [PATCH-V1] MdeModulePkg/NvmExpressDxe: Memory leak fix in async code flow Suman Prakash
@ 2017-03-21 2:08 ` Wu, Hao A
0 siblings, 0 replies; 2+ messages in thread
From: Wu, Hao A @ 2017-03-21 2:08 UTC (permalink / raw)
To: Suman Prakash, edk2-devel@lists.01.org; +Cc: Tian, Feng
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
^ permalink raw reply [flat|nested] 2+ messages in thread