Hi Neil,
On Tue, 8 Apr 2025 at 12:13, Neil Armstrong <neil.armstr...@linaro.org> 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.armstr...@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)
> +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",
> - 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;
> + }
> }
That's fine but since you'll send a v3 with a cleanup on top mind also
switching the logic around on this?
if (!blk_enabled())
return -ENOENT; etc
This will make it a bit more readable
> }
>
> 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()) {
Same here
if (!blk_enabled())
return;
etc
[...]
Other than that LGTM
Cheers
/Ilias
