[PATCH v2 4/6] lmb: remove superfluous address overlap check from lmb_add_region_flags()

[Sughosh Ganu]

On Thu, 20 Feb 2025 at 16:16, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 20.02.25 10:56, Sughosh Ganu wrote:
> > The lmb_reserve() function now does a check for a reservation request
> > with existing reserved regions, and returns -EEXIST in case of an
> > overlap. Remove this now redundant check from lmb_add_region_flags().
>
> We have numerous places where lmb_add_region_flags() is called,e.g.
> lmb_free_flags(), _lmb_alloc_base(). It is unclear to me if in all cases
> removing the check is allowable.

The reason this check had been put here was to handle the case of
request for the same memory region with a flag other than LMB_NONE.
With patch 1 of this series, this gets handled in lmb_reserve(). Also,
this will not be a scenario for lmb_free_flags() and
_lmb_alloc_base(). _lmb_free_flags() is freeing up memory, and
lmb_add_region_flags() gets called for adding a new memory region that
might be needed because the entire region has not been freed -- this
operation will not overlap with an existing region. Same for
_lmb_alloc_base(), as this function does not allocate existing in-use
memory -- it is designed to look for available memory. The
re-requesting of an existing region only happens through the
lmb_reserve() and lmb_alloc_addr() route.

>
> Could you, please, in the commit message provide an analysis for all
> callers that don't use LMB_NONE.

Will do. Thanks for your review.

-sughosh

>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V1: New patch
> >
> >   lib/lmb.c | 3 ---
> >   1 file changed, 3 deletions(-)
> >
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index 45888989457..061f9a07541 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -199,9 +199,6 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base,
> >                       coalesced++;
> >                       break;
> >               } else if (lmb_addrs_overlap(base, size, rgnbase, rgnsize)) {
> > -                     if (flags != LMB_NONE)
> > -                             return -EEXIST;
> > -
> >                       ret = lmb_resize_regions(lmb_rgn_lst, i, base, size);
> >                       if (ret < 0)
> >                               return -1;
>