Re: [PATCH v2 1/3] part: add part_get_info_cached() API

[Neil Armstrong <neil.armstrong@linaro.org>]

On 08/04/2025 11:55, Heinrich Schuchardt wrote:
> On 08.04.25 11:13, Neil Armstrong wrote:
>> Introduce a new part_get_info_cached() API that's used to
>> get the part_info of a blk_desc allowing to use an eventual
>> partition scanning cache to avoid rescanning the entire disk
>> partition scheme for each partition number.
>>
>> The part_get_info_cached_free() is also added to hint the
>> partition code to flush any cached data from this disk.
>>
>> The equivalent ops are added that directly maps to those
>> added functions.
>>
>> This API is designed to be used as a direct replacement of
>> part_get_info() in codes scanning all partitions for a same
>> disk, like in blk-uclass. With this, a lot of unnecessary
>> computation is saved, leading to a faster boot time when
>> partitions are scanned, especially with storage medias
>> with potentially multiple large hardware partitions like
>> UFS, NVMe or eMMC.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   disk/part.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
>>   include/part.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 88 insertions(+), 11 deletions(-)
>>
>> diff --git a/disk/part.c b/disk/part.c
>> index 303178161c083ec6e1b767b4f06ac5773576ca60..1d09c0511c75d457c81cab040c3f5caa924ee945 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -335,8 +335,8 @@ void part_print(struct blk_desc *desc)
>>           drv->print(desc);
>>   }
>>
>> -int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>> -              struct disk_partition *info)
> 
> Thank you, Neil, for pushing this forward.
> 
> All functions deserve a description.

Sure

> 
>> +static int _part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>> +                  struct disk_partition *info, bool cached)
>>   {
>>       struct part_driver *drv;
>>
>> @@ -356,24 +356,57 @@ int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>>                     desc->part_type);
>>               return -EPROTONOSUPPORT;
>>           }
>> -        if (!drv->get_info) {
>> -            PRINTF("## Driver %s does not have the get_info() method\n",
> 
> Writing to the console creates screen havoc in EFI applications. Please,
> use log_debug() instead of PRINTF().
> 
> And, please, remove that noisy '## '. It carries no information.

Right, those were already present, I'll add a cleanup patch before.

Thx,
Neil

> 
> Best regards
> 
> Heinrich
> 
>> -                   drv->name);
>> -            return -ENOSYS;
>> -        }
>> -        if (drv->get_info(desc, part, info) == 0) {
>> -            PRINTF("## Valid %s partition found ##\n", drv->name);
>> -            return 0;
>> +        if (cached && drv->get_info_cached) {
>> +            if (drv->get_info_cached(desc, part, info) == 0) {
>> +                PRINTF("## Valid %s partition found ##\n", drv->name);
>> +                return 0;
>> +            }
>> +        } else {
>> +            if (!drv->get_info) {
>> +                PRINTF("## Driver %s does not have the get_info() method\n",
>> +                       drv->name);
>> +                return -ENOSYS;
>> +            }
>> +            if (drv->get_info(desc, part, info) == 0) {
>> +                PRINTF("## Valid %s partition found ##\n", drv->name);
>> +                return 0;
>> +            }
>>           }
>>       }
>>
>>       return -ENOENT;
>>   }
>>
>> +int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>> +              struct disk_partition *info)
>> +{
>> +    return _part_get_info_by_type(desc, part, part_type, info, false);
>> +}
>> +
>> +int part_get_info_cached(struct blk_desc *desc, int part,
>> +             struct disk_partition *info)
>> +{
>> +    return _part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info, true);
>> +}
>> +
>>   int part_get_info(struct blk_desc *desc, int part,
>>             struct disk_partition *info)
>>   {
>> -    return part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info);
>> +    return _part_get_info_by_type(desc, part, PART_TYPE_UNKNOWN, info, false);
>> +}
>> +
>> +void part_get_info_cached_free(struct blk_desc *desc)
>> +{
>> +    struct part_driver *drv;
>> +
>> +    if (blk_enabled()) {
>> +        drv = part_driver_lookup_type(desc);
>> +        if (!drv)
>> +            return;
>> +        if (!drv->get_info_cache_free)
>> +            return;
>> +        drv->get_info_cache_free(desc);
>> +    }
>>   }
>>
>>   int part_get_info_whole_disk(struct blk_desc *desc,
>> diff --git a/include/part.h b/include/part.h
>> index fcb3c13dea4de6346ad98d6ce320ef36747dda85..8c98865146306fb66509576068c45de01b9cedb6 100644
>> --- a/include/part.h
>> +++ b/include/part.h
>> @@ -216,6 +216,34 @@ struct blk_desc *mg_disk_get_dev(int dev);
>>    */
>>   int part_get_info_by_type(struct blk_desc *desc, int part, int part_type,
>>                 struct disk_partition *info);
>> +
>> +/**
>> + * part_get_info_cached() - Get partitions from a block device but save the
>> + * disk partition map between different partitions.
>> + *
>> + * Call to part_get_info_cached_free() is required when scanning of the
>> + * block device is finished.
>> + *
>> + * If the partition driver doesn't support cached part_info, the
>> + * normal part_info will be called instead.
>> + *
>> + * @desc:    Block device descriptor
>> + * @part:    Partition number to read
>> + * @info:    Returned partition information
>> + *
>> + * Return: 0 on success, negative errno on failure
>> + */
>> +int part_get_info_cached(struct blk_desc *desc, int part,
>> +             struct disk_partition *info);
>> +
>> +/**
>> + * part_get_info_cached_free() - Free the saved disk partition map
>> + * from a previous call of part_get_info_cached().
>> + *
>> + * @desc:    Block device descriptor
>> + */
>> +void part_get_info_cached_free(struct blk_desc *desc);
>> +
>>   int part_get_info(struct blk_desc *desc, int part,
>>             struct disk_partition *info);
>>   /**
>> @@ -463,6 +491,22 @@ struct part_driver {
>>       int part_type;
>>       /** @max_entries:    maximum number of partition table entries */
>>       const int max_entries;
>> +    /**
>> +     * @cache_free_cache_free:    Free any parsing data stored from get_info_cached()
>> +     *
>> +     * @get_info_cache_free.desc:    Block device descriptor
>> +     */
>> +    void (*get_info_cache_free)(struct blk_desc *desc);
>> +    /**
>> +     * @get_inf_cachedo:        Get information about a partition, and save
>> +     *                the resulting parsing data for the next partition.
>> +     *
>> +     * @get_info_cached.desc:    Block device descriptor
>> +     * @get_info_cached.part:    Partition number (1 = first)
>> +     * @get_info_cached.info:    Returns partition information
>> +     */
>> +    int (*get_info_cached)(struct blk_desc *desc, int part,
>> +                   struct disk_partition *info);
>>       /**
>>        * @get_info:        Get information about a partition
>>        *
>>
>