1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot

Dandan Bi posted 1 patch 5 years ago
Failed in applying to current master (apply log)
.../Universal/HiiDatabaseDxe/Database.c       | 24 +++++++++++++------
1 file changed, 17 insertions(+), 7 deletions(-)
[edk2-devel] [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
Posted by Dandan Bi 5 years ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39535): https://edk2.groups.io/g/devel/message/39535
Mute This Topic: https://groups.io/mt/31338933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
Posted by Michael D Kinney 5 years ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39582): https://edk2.groups.io/g/devel/message/39582
Mute This Topic: https://groups.io/mt/31338933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot
Posted by Dong, Eric 5 years ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39587): https://edk2.groups.io/g/devel/message/39587
Mute This Topic: https://groups.io/mt/31338933/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.