| Message ID | 20250220095654.121634-7-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | lmb: miscellaneous fixes and improvements | expand |
On 20.02.25 10:56, Sughosh Ganu wrote: > The actual logic to allocate a region of memory is in the > _lmb_alloc_base() function. The lmb_alloc() API function calls > lmb_alloc_base(), which then calls _lmb_alloc_base() to do the > allocation. Instead, call the _lmb_alloc_base() directly from both the > allocation API's, and move the error message to the _lmb_alloc_base(). > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V1: New patch > > lib/lmb.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/lib/lmb.c b/lib/lmb.c > index 874063fc1f5..6bfc0dbc9ce 100644 > --- a/lib/lmb.c > +++ b/lib/lmb.c > @@ -758,26 +758,22 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, > base = ALIGN_DOWN(res_base - size, align); > } > } > + > + log_err("Failed to allocate 0x%lx bytes below 0x%lx\n", (ulong)size, > + (ulong)max_addr); Printing such a line is not expected in an EFI application. It would mess up the output of menus in EFI applications like GRUB. I think this line should be debug output. Best regards Heinrich > + > return 0; > } > > phys_addr_t lmb_alloc(phys_size_t size, ulong align) > { > - return lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); > + return _lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); > } > > phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, > uint flags) > { > - phys_addr_t alloc; > - > - alloc = _lmb_alloc_base(size, align, max_addr, flags); > - > - if (alloc == 0) > - printf("ERROR: Failed to allocate 0x%lx bytes below 0x%lx.\n", > - (ulong)size, (ulong)max_addr); > - > - return alloc; > + return _lmb_alloc_base(size, align, max_addr, flags); > } > > phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size, u32 flags)
On Thu, 20 Feb 2025 at 16:20, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > On 20.02.25 10:56, Sughosh Ganu wrote: > > The actual logic to allocate a region of memory is in the > > _lmb_alloc_base() function. The lmb_alloc() API function calls > > lmb_alloc_base(), which then calls _lmb_alloc_base() to do the > > allocation. Instead, call the _lmb_alloc_base() directly from both the > > allocation API's, and move the error message to the _lmb_alloc_base(). > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V1: New patch > > > > lib/lmb.c | 16 ++++++---------- > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > diff --git a/lib/lmb.c b/lib/lmb.c > > index 874063fc1f5..6bfc0dbc9ce 100644 > > --- a/lib/lmb.c > > +++ b/lib/lmb.c > > @@ -758,26 +758,22 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, > > base = ALIGN_DOWN(res_base - size, align); > > } > > } > > + > > + log_err("Failed to allocate 0x%lx bytes below 0x%lx\n", (ulong)size, > > + (ulong)max_addr); > > Printing such a line is not expected in an EFI application. It would > mess up the output of menus in EFI applications like GRUB. > > I think this line should be debug output. Okay, I will change it to a log_debug(). Since this was getting printed in the current code, I thought of simply moving it to a more apt place. -sughosh > > Best regards > > Heinrich > > > + > > return 0; > > } > > > > phys_addr_t lmb_alloc(phys_size_t size, ulong align) > > { > > - return lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); > > + return _lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); > > } > > > > phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, > > uint flags) > > { > > - phys_addr_t alloc; > > - > > - alloc = _lmb_alloc_base(size, align, max_addr, flags); > > - > > - if (alloc == 0) > > - printf("ERROR: Failed to allocate 0x%lx bytes below 0x%lx.\n", > > - (ulong)size, (ulong)max_addr); > > - > > - return alloc; > > + return _lmb_alloc_base(size, align, max_addr, flags); > > } > > > > phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size, u32 flags) >
diff --git a/lib/lmb.c b/lib/lmb.c index 874063fc1f5..6bfc0dbc9ce 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -758,26 +758,22 @@ static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, base = ALIGN_DOWN(res_base - size, align); } } + + log_err("Failed to allocate 0x%lx bytes below 0x%lx\n", (ulong)size, + (ulong)max_addr); + return 0; } phys_addr_t lmb_alloc(phys_size_t size, ulong align) { - return lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); + return _lmb_alloc_base(size, align, LMB_ALLOC_ANYWHERE, LMB_NONE); } phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, uint flags) { - phys_addr_t alloc; - - alloc = _lmb_alloc_base(size, align, max_addr, flags); - - if (alloc == 0) - printf("ERROR: Failed to allocate 0x%lx bytes below 0x%lx.\n", - (ulong)size, (ulong)max_addr); - - return alloc; + return _lmb_alloc_base(size, align, max_addr, flags); } phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size, u32 flags)
The actual logic to allocate a region of memory is in the _lmb_alloc_base() function. The lmb_alloc() API function calls lmb_alloc_base(), which then calls _lmb_alloc_base() to do the allocation. Instead, call the _lmb_alloc_base() directly from both the allocation API's, and move the error message to the _lmb_alloc_base(). Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V1: New patch lib/lmb.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)