| Message ID | 20250408-topic-gpt-cache-v2-1-de76e47debb7@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | part_efi: cache last scanned GPT for next partition | expand |
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. > +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. 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 > * >
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 >> * >> >
Hi Neil, On Tue, 8 Apr 2025 at 12:13, Neil Armstrong <neil.armstrong@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.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) > +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
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; + } } } 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 *
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(-)