Re: [RFC PATCH 03/31] lmb: make the lmb reservations persistent

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 1606 bytes --]

On Mon, Jun 10, 2024 at 01:23:49PM +0200, Heinrich Schuchardt wrote:
> On 1/1/70 01:00, Ilias Apalodimas wrote:
> > Hi Sughosh
> > 
> > [...]
> > 
> > >   #define LMB_ALLOC_ANYWHERE	0
> > > 
> > > +#if !IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> > > +struct lmb_property memory_regions[CONFIG_LMB_MEMORY_REGIONS];
> > > +struct lmb_property reserved_regions[CONFIG_LMB_RESERVED_REGIONS];
> > > +#endif
> > > +
> > > +struct lmb lmb = {
> > > +#if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> > > +	.memory.max = CONFIG_LMB_MAX_REGIONS,
> > > +	.reserved.max = CONFIG_LMB_MAX_REGIONS,
> > > +#else
> > > +	.memory.max = CONFIG_LMB_MEMORY_REGIONS,
> > > +	.reserved.max = CONFIG_LMB_RESERVED_REGIONS,
> > > +	.memory.region = memory_regions,
> > > +	.reserved.region = reserved_regions,
> > 
> > This is probably a good opportunity to look into why
> > CONFIG_LMB_MEMORY_REGIONS was introduced.  Since we are moving towards
> > static allocations, do we still need it? Or allocating the size dynamically
> > covers all our cases.
> 
> Up to now we used static arrays for saving memory allocations in LMB:
> 
> include/lmb.h:67:
> struct lmb_property region[CONFIG_LMB_MAX_REGIONS];
> 
> As the EFI sub-system can produce any number of non-coalescable memory
> regions we should use a linked list instead.

I think it's some historic flexibility that's I believe no longer really
relevant to how we use LMB today, let alone once this patch series is
complete. We should probably (I'm doing my size check thing now..) move
to just following Heinrich's suggestion.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]