Hi Shiji,
On 2026-10-13T15:04:41, Shiji Yang <[email protected]> wrote:
> cmd/meminfo: fix top lmb information
>
> Always assume that the top lmb is the u-boot region. Set *uptop to
> 0 to avoid printing meaningless Gap size, as it ends at the end of
> memory, just like the `code` region.
Please can you show the 'meminfo' output before and after, so it is
clear what is being fixed. Also please add a Fixes: tag for
9252b7f867f ("meminfo: Show the lmb records"), which is where the
broken condition comes from.
>
> Signed-off-by: Shiji Yang <[email protected]>
>
> cmd/meminfo.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
> diff --git a/cmd/meminfo.c b/cmd/meminfo.c
> @@ -40,15 +40,14 @@ static void show_lmb(const struct lmb *lmb, ulong *uptop)
> - /*
> - * Assume that the top lmb region is the U-Boot region, so just
> - * take account of the memory not already reported
> - */
> - if (lmb->used_mem.count - 1)
> + /* Assume that the top lmb region is the U-Boot region */
> + if (i == lmb->used_mem.count - 1) {
> + *uptop = 0;
> + print_region("lmb", rgn->base, rgn->size, uptop);
> + } else {
> print_region("lmb", rgn->base, *uptop - rgn->base,
> uptop);
> - else
> - print_region("lmb", rgn->base, rgn->size, uptop);
> + }
You are right that the condition is wrong, but I think the two
branches are now the wrong way round. lmb_reserve_uboot_region()
reserves from the stack bottom up to initial_relocaddr, so the top lmb
region covers everything already printed (stack, bloblist, ..., code,
video). The intent of the comment you removed is that only the part of
that region below what is already reported gets shown, i.e. '*uptop -
rgn->base' for the top region and 'rgn->size' for all the others.
With this patch the top region shows its full size, overlapping all
the lines above it, and its gap is hidden. Every other lmb region gets
a size that runs up to the base of the region above it, so its gap is
always 0 and any real holes between reservations disappear.
I think the fix is just:
if (i == lmb->used_mem.count - 1)
print_region('lmb', rgn->base, *uptop - rgn->base,
uptop);
else
print_region('lmb', rgn->base, rgn->size, uptop);
and you can keep the original comment. What do you think?
BTW the '*uptop = rgn->base' after this is redundant, since
print_region() already does it. It would be worth dropping it while
you are here.
Regards,
Simon
