Re: [PATCH v2 2/3] part: efi: add GPT PTE cache used with part_get_info_cached() API

Tue, 08 Apr 2025 07:53:04 -0700 

On 08/04/2025 12:57, Heinrich Schuchardt wrote:

On 08.04.25 11:13, Neil Armstrong wrote:

Implement a simple cache for part_efi to be used by the newly
introduced part_get_info_cached() API.

The cache simply stores a successfully scanned GPT PTE if called
from the part_get_info_cached() ops, and will return the cached
data if the blk_desc and lba offset matches, invalidating the
cache if not.

The cache will only operate if called from the part_get_info_cached()
API, all the over calls will operate uncached, making sure we
can still update & re-scan the GPT partitions like before.

Signed-off-by: Neil Armstrong <neil.armstr...@linaro.org>
---
  disk/part_efi.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 96 insertions(+), 18 deletions(-)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 
932d058c184ce6946b7142e7c2d9637b28e4661e..949968fcd30fc18766a023e73f43ca1381dcef89
 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -55,12 +55,58 @@ static inline u32 efi_crc32(const void *buf, u32 len)
  static int pmbr_part_valid(struct partition *part);
  static int is_pmbr_valid(legacy_mbr * mbr);
  static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
-            gpt_entry **pgpt_pte);
+            gpt_entry **pgpt_pte, bool cache);
  static gpt_entry *alloc_read_gpt_entries(struct blk_desc *desc,
                       gpt_header *pgpt_head);
  static int is_pte_valid(gpt_entry * pte);
  static int find_valid_gpt(struct blk_desc *desc, gpt_header *gpt_head,
-              gpt_entry **pgpt_pte);
+              gpt_entry **pgpt_pte, bool cache);
+


Please, describe the structure and its members.


Ack



https://docs.kernel.org/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation


+static struct gpt_pte_cache_data {
+    struct blk_desc *desc;
+    u64 lba;
+    gpt_entry *gpt_pte;
+    gpt_header gpt_head;
+} gpt_pte_cache;
+


Please, describe all functions.


Ack




+static void clear_gpt_pte_cache(void)
+{
+    if (gpt_pte_cache.desc) {
+        if (gpt_pte_cache.gpt_pte)
+            free(gpt_pte_cache.gpt_pte);
+
+        memset(&gpt_pte_cache, 0, sizeof(gpt_pte_cache));
+    }
+}
+
+static void cache_gpt_pte(struct blk_desc *desc, u64 lba,
+              gpt_entry *gpt_pte, gpt_header *pgpt_head)
+{
+    if (gpt_pte_cache.gpt_pte)
+        free(gpt_pte_cache.gpt_pte);
+
+    gpt_pte_cache.desc = desc;
+    gpt_pte_cache.lba = lba;
+    gpt_pte_cache.gpt_pte = gpt_pte;
+    if (pgpt_head)
+        memcpy(&gpt_pte_cache.gpt_head, pgpt_head, sizeof(gpt_header));
+}
+
+static gpt_entry *get_cached_gpt_pte(struct blk_desc *desc, u64 lba,
+                     gpt_header *pgpt_head)
+{
+    if (gpt_pte_cache.desc && gpt_pte_cache.gpt_pte) {
+        if (gpt_pte_cache.desc == desc &&
+            gpt_pte_cache.lba == lba) {
+            memcpy(pgpt_head, &gpt_pte_cache.gpt_head, sizeof(gpt_header));
+            return gpt_pte_cache.gpt_pte;
+        }
+
+        clear_gpt_pte_cache();
+    }
+
+    return NULL;
+}

  static char *print_efiname(gpt_entry *pte)
  {
@@ -205,7 +251,7 @@ int get_disk_guid(struct blk_desc *desc, char *guid)
      unsigned char *guid_bin;

      /* This function validates AND fills in the GPT header and PTE */
-    if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
+    if (find_valid_gpt(desc, gpt_head, &gpt_pte, false) != 1)
          return -EINVAL;

      guid_bin = gpt_head->disk_guid.b;
@@ -224,7 +270,7 @@ static void __maybe_unused part_print_efi(struct blk_desc 
*desc)
      unsigned char *uuid;

      /* This function validates AND fills in the GPT header and PTE */
-    if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
+    if (find_valid_gpt(desc, gpt_head, &gpt_pte, false) != 1)
          return;

      debug("%s: gpt-entry at %p\n", __func__, gpt_pte);
@@ -258,8 +304,13 @@ static void __maybe_unused part_print_efi(struct blk_desc 
*desc)
      return;
  }

-static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
-                        struct disk_partition *info)
+static void __maybe_unused part_get_info_cache_free_efi(struct blk_desc *desc)
+{
+    clear_gpt_pte_cache();
+}
+
+static int _part_get_info_efi(struct blk_desc *desc, int part,
+                  struct disk_partition *info, bool cache)
  {
      ALLOC_CACHE_ALIGN_BUFFER_PAD(gpt_header, gpt_head, 1, desc->blksz);
      gpt_entry *gpt_pte = NULL;
@@ -271,13 +322,14 @@ static int __maybe_unused part_get_info_efi(struct 
blk_desc *desc, int part,
      }

      /* This function validates AND fills in the GPT header and PTE */
-    if (find_valid_gpt(desc, gpt_head, &gpt_pte) != 1)
+    if (find_valid_gpt(desc, gpt_head, &gpt_pte, cache) != 1)
          return -EINVAL;

      if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
          !is_pte_valid(&gpt_pte[part - 1])) {
          log_debug("Invalid partition number %d\n", part);
-        free(gpt_pte);
+        if (!cache)
+            free(gpt_pte);
          return -EPERM;
      }

@@ -307,11 +359,23 @@ static int __maybe_unused part_get_info_efi(struct 
blk_desc *desc, int part,
      log_debug("start 0x" LBAF ", size 0x" LBAF ", name %s\n", info->start,
            info->size, info->name);

-    /* Remember to free pte */
-    free(gpt_pte);
+    if (!cache)
+        free(gpt_pte);
      return 0;
  }

+static int __maybe_unused part_get_info_cached_efi(struct blk_desc *desc, int 
part,
+                           struct disk_partition *info)
+{
+    return _part_get_info_efi(desc, part, info, true);
+}
+
+static int __maybe_unused part_get_info_efi(struct blk_desc *desc, int part,
+                        struct disk_partition *info)
+{
+    return _part_get_info_efi(desc, part, info, false);
+}
+
  static int part_test_efi(struct blk_desc *desc)
  {
      ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, legacymbr, 1, desc->blksz);
@@ -689,7 +753,7 @@ int gpt_verify_headers(struct blk_desc *desc, gpt_header 
*gpt_head,
       */
      if (is_gpt_valid(desc,
               GPT_PRIMARY_PARTITION_TABLE_LBA,
-             gpt_head, gpt_pte) != 1) {
+             gpt_head, gpt_pte, false) != 1) {
          log_debug("Invalid GPT\n");
          return -1;
      }
@@ -706,7 +770,7 @@ int gpt_verify_headers(struct blk_desc *desc, gpt_header 
*gpt_head,
      }

      if (is_gpt_valid(desc, (desc->lba - 1),
-             gpt_head, gpt_pte) != 1) {
+             gpt_head, gpt_pte, false) != 1) {
          log_debug("Invalid Backup GPT\n");
          return -1;
      }
@@ -765,9 +829,9 @@ int gpt_repair_headers(struct blk_desc *desc)
      int ret = -1;

      is_gpt1_valid = is_gpt_valid(desc, GPT_PRIMARY_PARTITION_TABLE_LBA,
-                     gpt_h1, &gpt_e1);
+                     gpt_h1, &gpt_e1, false);
      is_gpt2_valid = is_gpt_valid(desc, desc->lba - 1,
-                     gpt_h2, &gpt_e2);
+                     gpt_h2, &gpt_e2, false);

      if (is_gpt1_valid && is_gpt2_valid) {
          ret = 0;
@@ -1023,12 +1087,13 @@ static int is_pmbr_valid(legacy_mbr *mbr)
   * lba is the logical block address of the GPT header to test
   * gpt is a GPT header ptr, filled on return.
   * ptes is a PTEs ptr, filled on return.
+ * cache is a bool, true to use the cached gpt_pte from previous call


"use the cache" or "use cached GPT if available"?

It remains unclear what will happen if cache=1 and no entry is found.


Ack, will clarify



Best regards

Heinrich


   *
   * Description: returns 1 if valid,  0 on error, 2 if ignored header
   * If valid, returns pointers to PTEs.
   */
  static int is_gpt_valid(struct blk_desc *desc, u64 lba, gpt_header *pgpt_head,
-            gpt_entry **pgpt_pte)
+            gpt_entry **pgpt_pte, bool cache)
  {
      /* Confirm valid arguments prior to allocation. */
      if (!desc || !pgpt_head) {
@@ -1036,6 +1101,12 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, 
gpt_header *pgpt_head,
          return 0;
      }

+    if (cache) {
+        *pgpt_pte = get_cached_gpt_pte(desc, lba, pgpt_head);
+        if (*pgpt_pte)
+            return 1;
+    }
+
      ALLOC_CACHE_ALIGN_BUFFER_PAD(legacy_mbr, mbr, 1, desc->blksz);

      /* Read MBR Header from device */
@@ -1081,6 +1152,9 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, 
gpt_header *pgpt_head,
          return 0;
      }

+    if (cache)
+        cache_gpt_pte(desc, lba, *pgpt_pte, pgpt_head);
+
      /* We're done, all's well */
      return 1;
  }
@@ -1090,23 +1164,25 @@ static int is_gpt_valid(struct blk_desc *desc, u64 lba, 
gpt_header *pgpt_head,
   *
   * gpt is a GPT header ptr, filled on return.
   * ptes is a PTEs ptr, filled on return.
+ * cache if a bool, use cached GPT if available
   *
   * Description: returns 1 if found a valid gpt,  0 on error.
   * If valid, returns pointers to PTEs.
   */
  static int find_valid_gpt(struct blk_desc *desc, gpt_header *gpt_head,
-              gpt_entry **pgpt_pte)
+              gpt_entry **pgpt_pte, bool cache)
  {
      int r;

      r = is_gpt_valid(desc, GPT_PRIMARY_PARTITION_TABLE_LBA, gpt_head,
-             pgpt_pte);
+             pgpt_pte, cache);

      if (r != 1) {
          if (r != 2)
              log_debug("Invalid GPT\n");

-        if (is_gpt_valid(desc, desc->lba - 1, gpt_head, pgpt_pte)
+        if (is_gpt_valid(desc, desc->lba - 1, gpt_head, pgpt_pte,
+                 true)
              != 1) {
              log_debug("Invalid Backup GPT\n");
              return 0;
@@ -1210,6 +1286,8 @@ U_BOOT_PART_TYPE(a_efi) = {
      .name        = "EFI",
      .part_type    = PART_TYPE_EFI,
      .max_entries    = GPT_ENTRY_NUMBERS,
+    .get_info_cache_free    = part_get_info_ptr(part_get_info_cache_free_efi),
+    .get_info_cached    = part_get_info_ptr(part_get_info_cached_efi),
      .get_info    = part_get_info_ptr(part_get_info_efi),
      .print        = part_print_ptr(part_print_efi),
      .test        = part_test_efi,

Reply via email to