On Thu, 20 Feb 2025 at 16:16, Heinrich Schuchardt <[email protected]> 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 <[email protected]> > > --- > > 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; >
