Hi Neil,
On Tue, 8 Apr 2025 at 12:13, Neil Armstrong <neil.armstr...@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.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);
> +
> +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
