* [PATCH-V1] MdeModulePkg/NvmExpressDxe: Memory leak fix in async code flow
[not found] <CGME20170320083513epcas1p3296f5fef6610fa19af50669a1465a669@epcas1p3.samsung.com>
@ 2017-03-20 8:34 ` Suman Prakash
2017-03-21 2:08 ` Wu, Hao A
0 siblings, 1 reply; 2+ messages in thread
From: Suman Prakash @ 2017-03-20 8:34 UTC (permalink / raw)
To: edk2-devel; +Cc: feng.tian, Suman Prakash
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* 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
end of thread, other threads:[~2017-03-21 2:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20170320083513epcas1p3296f5fef6610fa19af50669a1465a669@epcas1p3.samsung.com>
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox