Re: [PATCH 4/9] kern/ieee1275/init: ppc64: Check early for sufficiently large chunk

[Stefan Berger]

On 11/8/23 11:11, Daniel Kiper wrote:
> On Tue, Oct 31, 2023 at 01:50:26PM -0400, Stefan Berger wrote:
> > If a single memory block is requested (GRUB_MM_ADD_REGION_CONSECUTIVE)
> > then check early on whether the given block from the memory map is
> > big enough to satisfy the request.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > Cc: Pavithra Prakash <pavrampu@in.ibm.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Carolyn Scherrer <cpscherr@us.ibm.com>
> > Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> > Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
> > ---
> >   grub-core/kern/ieee1275/init.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> > index e09db4192..a0ce9d55e 100644
> > --- a/grub-core/kern/ieee1275/init.c
> > +++ b/grub-core/kern/ieee1275/init.c
> > @@ -328,6 +328,10 @@ regions_claim (grub_uint64_t addr, grub_uint64_t len, 
> > grub_memory_type_t type,
> >     if (type != GRUB_MEMORY_AVAILABLE)
> >       return 0;
> >
> > +  /* If a specific size was requested: is this chunk big enough? */
> > +  if (rcr->flags & GRUB_MM_ADD_REGION_CONSECUTIVE && len < rcr->total)
> > +    return 0;
>
> Do we really need that? Same check is a few lines below. If we need both
> checks the commit message should explain why. And I would add comments
> before ifs explaining what they do and why they are needed/different/...

I could drop this one. It's an optimization to not even look at a free 
memory chunk if it's too small to fulfill the request just to begin 
with. The check later on will catch it then as well.

  Stefan

> Daniel