| Message ID | 20250220095654.121634-5-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 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. Could you, please, in the commit message provide an analysis for all callers that don't use LMB_NONE. Best regards Heinrich > > Signed-off-by: Sughosh Ganu <sughosh.ganu@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;
On Thu, 20 Feb 2025 at 16:16, Heinrich Schuchardt <heinrich.schuchardt@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@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; >
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;
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(). Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V1: New patch lib/lmb.c | 3 --- 1 file changed, 3 deletions(-)