* [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
@ 2019-04-25 3:37 Dandan Bi
2019-04-25 22:33 ` Michael D Kinney
2019-04-26 0:07 ` Dong, Eric
0 siblings, 2 replies; 3+ messages in thread
From: Dandan Bi @ 2019-04-25 3:37 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Eric Dong
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1597
Currently RTData are allocated at/after ReadyToBoot to store the
contents in HiiDatabase and the HII configurations for OS runtime
utilization.
Some platforms may meet S4 resume issue since the allocation after
ReadyToBoot cause memory map change.
Now this patch to do some overallocation to minimize the number
of memory allocations after ReadyToBoot and also add warning
message when do allocation after ReadyToBoot.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
.../Universal/HiiDatabaseDxe/Database.c | 24 +++++++++++++------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
index 6da0e30c98..d3791ca68b 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
@@ -1,9 +1,9 @@
/** @file
Implementation for EFI_HII_DATABASE_PROTOCOL.
-Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -3361,18 +3361,23 @@ HiiGetConfigRespInfo(
Status = HiiConfigRoutingExportConfig(&Private->ConfigRouting,&ConfigAltResp);
if (!EFI_ERROR (Status)){
ConfigSize = StrSize(ConfigAltResp);
if (ConfigSize > gConfigRespSize){
- gConfigRespSize = ConfigSize;
+ //
+ // Do 25% overallocation to minimize the number of memory allocations after ReadyToBoot.
+ // Since lots of allocation after ReadyToBoot may change memory map and cause S4 resume issue.
+ //
+ gConfigRespSize = ConfigSize + (ConfigSize >> 2);
if (gRTConfigRespBuffer != NULL){
FreePool(gRTConfigRespBuffer);
+ DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required after ReadyToBoot, which may change memory map and cause S4 resume issue.\n"));
}
- gRTConfigRespBuffer = (EFI_STRING)AllocateRuntimeZeroPool(ConfigSize);
+ gRTConfigRespBuffer = (EFI_STRING) AllocateRuntimeZeroPool (gConfigRespSize);
if (gRTConfigRespBuffer == NULL){
FreePool(ConfigAltResp);
- DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the ConfigResp string.\n"));
+ DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource to store the ConfigResp string.\n"));
return EFI_OUT_OF_RESOURCES;
}
} else {
ZeroMem(gRTConfigRespBuffer,gConfigRespSize);
}
@@ -3412,17 +3417,22 @@ HiiGetDatabaseInfo(
Status = HiiExportPackageLists(This, NULL, &DatabaseInfoSize, DatabaseInfo);
ASSERT(Status == EFI_BUFFER_TOO_SMALL);
if(DatabaseInfoSize > gDatabaseInfoSize ) {
- gDatabaseInfoSize = DatabaseInfoSize;
+ //
+ // Do 25% overallocation to minimize the number of memory allocations after ReadyToBoot.
+ // Since lots of allocation after ReadyToBoot may change memory map and cause S4 resume issue.
+ //
+ gDatabaseInfoSize = DatabaseInfoSize + (DatabaseInfoSize >> 2);
if (gRTDatabaseInfoBuffer != NULL){
FreePool(gRTDatabaseInfoBuffer);
+ DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required after ReadyToBoot, which may change memory map and cause S4 resume issue.\n"));
}
- gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool(DatabaseInfoSize);
+ gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool (gDatabaseInfoSize);
if (gRTDatabaseInfoBuffer == NULL){
- DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the HiiDatabase info.\n"));
+ DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource to store the HiiDatabase info.\n"));
return EFI_OUT_OF_RESOURCES;
}
} else {
ZeroMem(gRTDatabaseInfoBuffer,gDatabaseInfoSize);
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
2019-04-25 3:37 [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot Dandan Bi
@ 2019-04-25 22:33 ` Michael D Kinney
2019-04-26 0:07 ` Dong, Eric
1 sibling, 0 replies; 3+ messages in thread
From: Michael D Kinney @ 2019-04-25 22:33 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io, Kinney, Michael D
Cc: Gao, Liming, Dong, Eric
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: Bi, Dandan
> Sent: Wednesday, April 24, 2019 8:37 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming <liming.gao@intel.com>; Dong, Eric
> <eric.dong@intel.com>
> Subject: [patch] MdeModulePkg/HiiDB: Minimize memory
> allocation times after ReadyToBoot
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1597
>
> Currently RTData are allocated at/after ReadyToBoot to
> store the
> contents in HiiDatabase and the HII configurations for OS
> runtime
> utilization.
> Some platforms may meet S4 resume issue since the
> allocation after
> ReadyToBoot cause memory map change.
> Now this patch to do some overallocation to minimize the
> number
> of memory allocations after ReadyToBoot and also add
> warning
> message when do allocation after ReadyToBoot.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
> .../Universal/HiiDatabaseDxe/Database.c | 24
> +++++++++++++------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index 6da0e30c98..d3791ca68b 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> @@ -1,9 +1,9 @@
> /** @file
> Implementation for EFI_HII_DATABASE_PROTOCOL.
>
> -Copyright (c) 2007 - 2018, Intel Corporation. All rights
> reserved.<BR>
> +Copyright (c) 2007 - 2019, Intel Corporation. All rights
> reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
>
> @@ -3361,18 +3361,23 @@ HiiGetConfigRespInfo(
> Status = HiiConfigRoutingExportConfig(&Private-
> >ConfigRouting,&ConfigAltResp);
>
> if (!EFI_ERROR (Status)){
> ConfigSize = StrSize(ConfigAltResp);
> if (ConfigSize > gConfigRespSize){
> - gConfigRespSize = ConfigSize;
> + //
> + // Do 25% overallocation to minimize the number of
> memory allocations after ReadyToBoot.
> + // Since lots of allocation after ReadyToBoot may
> change memory map and cause S4 resume issue.
> + //
> + gConfigRespSize = ConfigSize + (ConfigSize >> 2);
> if (gRTConfigRespBuffer != NULL){
> FreePool(gRTConfigRespBuffer);
> + DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory
> allocation is required after ReadyToBoot, which may
> change memory map and cause S4 resume issue.\n"));
> }
> - gRTConfigRespBuffer =
> (EFI_STRING)AllocateRuntimeZeroPool(ConfigSize);
> + gRTConfigRespBuffer = (EFI_STRING)
> AllocateRuntimeZeroPool (gConfigRespSize);
> if (gRTConfigRespBuffer == NULL){
> FreePool(ConfigAltResp);
> - DEBUG ((DEBUG_ERROR, "Not enough memory resource
> to get the ConfigResp string.\n"));
> + DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough
> memory resource to store the ConfigResp string.\n"));
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTConfigRespBuffer,gConfigRespSize);
> }
> @@ -3412,17 +3417,22 @@ HiiGetDatabaseInfo(
> Status = HiiExportPackageLists(This, NULL,
> &DatabaseInfoSize, DatabaseInfo);
>
> ASSERT(Status == EFI_BUFFER_TOO_SMALL);
>
> if(DatabaseInfoSize > gDatabaseInfoSize ) {
> - gDatabaseInfoSize = DatabaseInfoSize;
> + //
> + // Do 25% overallocation to minimize the number of
> memory allocations after ReadyToBoot.
> + // Since lots of allocation after ReadyToBoot may
> change memory map and cause S4 resume issue.
> + //
> + gDatabaseInfoSize = DatabaseInfoSize +
> (DatabaseInfoSize >> 2);
> if (gRTDatabaseInfoBuffer != NULL){
> FreePool(gRTDatabaseInfoBuffer);
> + DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory
> allocation is required after ReadyToBoot, which may
> change memory map and cause S4 resume issue.\n"));
> }
> - gRTDatabaseInfoBuffer =
> AllocateRuntimeZeroPool(DatabaseInfoSize);
> + gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool
> (gDatabaseInfoSize);
> if (gRTDatabaseInfoBuffer == NULL){
> - DEBUG ((DEBUG_ERROR, "Not enough memory resource
> to get the HiiDatabase info.\n"));
> + DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough
> memory resource to store the HiiDatabase info.\n"));
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTDatabaseInfoBuffer,gDatabaseInfoSize);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
2019-04-25 3:37 [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot Dandan Bi
2019-04-25 22:33 ` Michael D Kinney
@ 2019-04-26 0:07 ` Dong, Eric
1 sibling, 0 replies; 3+ messages in thread
From: Dong, Eric @ 2019-04-26 0:07 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Kinney, Michael D, Gao, Liming
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: Bi, Dandan
> Sent: Thursday, April 25, 2019 11:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [patch] MdeModulePkg/HiiDB: Minimize memory allocation times
> after ReadyToBoot
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1597
>
> Currently RTData are allocated at/after ReadyToBoot to store the
> contents in HiiDatabase and the HII configurations for OS runtime
> utilization.
> Some platforms may meet S4 resume issue since the allocation after
> ReadyToBoot cause memory map change.
> Now this patch to do some overallocation to minimize the number
> of memory allocations after ReadyToBoot and also add warning
> message when do allocation after ReadyToBoot.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
> .../Universal/HiiDatabaseDxe/Database.c | 24 +++++++++++++------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index 6da0e30c98..d3791ca68b 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> @@ -1,9 +1,9 @@
> /** @file
> Implementation for EFI_HII_DATABASE_PROTOCOL.
>
> -Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
>
> @@ -3361,18 +3361,23 @@ HiiGetConfigRespInfo(
> Status = HiiConfigRoutingExportConfig(&Private-
> >ConfigRouting,&ConfigAltResp);
>
> if (!EFI_ERROR (Status)){
> ConfigSize = StrSize(ConfigAltResp);
> if (ConfigSize > gConfigRespSize){
> - gConfigRespSize = ConfigSize;
> + //
> + // Do 25% overallocation to minimize the number of memory allocations
> after ReadyToBoot.
> + // Since lots of allocation after ReadyToBoot may change memory map
> and cause S4 resume issue.
> + //
> + gConfigRespSize = ConfigSize + (ConfigSize >> 2);
> if (gRTConfigRespBuffer != NULL){
> FreePool(gRTConfigRespBuffer);
> + DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required
> after ReadyToBoot, which may change memory map and cause S4 resume
> issue.\n"));
> }
> - gRTConfigRespBuffer =
> (EFI_STRING)AllocateRuntimeZeroPool(ConfigSize);
> + gRTConfigRespBuffer = (EFI_STRING) AllocateRuntimeZeroPool
> (gConfigRespSize);
> if (gRTConfigRespBuffer == NULL){
> FreePool(ConfigAltResp);
> - DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the
> ConfigResp string.\n"));
> + DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource
> to store the ConfigResp string.\n"));
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTConfigRespBuffer,gConfigRespSize);
> }
> @@ -3412,17 +3417,22 @@ HiiGetDatabaseInfo(
> Status = HiiExportPackageLists(This, NULL, &DatabaseInfoSize,
> DatabaseInfo);
>
> ASSERT(Status == EFI_BUFFER_TOO_SMALL);
>
> if(DatabaseInfoSize > gDatabaseInfoSize ) {
> - gDatabaseInfoSize = DatabaseInfoSize;
> + //
> + // Do 25% overallocation to minimize the number of memory allocations
> after ReadyToBoot.
> + // Since lots of allocation after ReadyToBoot may change memory map
> and cause S4 resume issue.
> + //
> + gDatabaseInfoSize = DatabaseInfoSize + (DatabaseInfoSize >> 2);
> if (gRTDatabaseInfoBuffer != NULL){
> FreePool(gRTDatabaseInfoBuffer);
> + DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required
> after ReadyToBoot, which may change memory map and cause S4 resume
> issue.\n"));
> }
> - gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool(DatabaseInfoSize);
> + gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool (gDatabaseInfoSize);
> if (gRTDatabaseInfoBuffer == NULL){
> - DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the
> HiiDatabase info.\n"));
> + DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource
> to store the HiiDatabase info.\n"));
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTDatabaseInfoBuffer,gDatabaseInfoSize);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-26 0:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-25 3:37 [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot Dandan Bi
2019-04-25 22:33 ` Michael D Kinney
2019-04-26 0:07 ` Dong, Eric
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox