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

[Ilias Apalodimas]

Hi Neil,


On Tue, 8 Apr 2025 at 12:13, Neil Armstrong <neil.armstrong at linaro.org> 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.armstrong at 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);
> +
> +static struct gpt_pte_cache_data {
> +       struct blk_desc *desc;
> +       u64 lba;
> +       gpt_entry *gpt_pte;
> +       gpt_header gpt_head;
> +} gpt_pte_cache;

My only concern is that this is global, and I don't know how it will
affect the caching with the threading framework that's under
discussion.
I don't know the block framework well enough, but it would make some
sense to store this per block device eventually.

> +
> +static void clear_gpt_pte_cache(void)
> +{
> +       if (gpt_pte_cache.desc) {
> +               if (gpt_pte_cache.gpt_pte)
> +                       free(gpt_pte_cache.gpt_pte);
> +
[...]

Other than that this looks sane to me

Thanks
/Ilias