Message ID | 20221019120337.2098298-1-yajun.deng@linux.dev (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | memblock: remove repeat round | expand |
Hi, On Wed, Oct 19, 2022 at 08:03:37PM +0800, Yajun Deng wrote: > Subject: memblock: remove repeat round Please make the patch subject more detailed. Say membloc: don't run loop in memblock_add_range() twice > There is no need round twice in memblock_add_range(). > > We can call memblock_double_array() to extand the size if type->cnt no ^ extend > less than type->max before memblock_insert_region(), otherwise we can s/no less than/greater or equal to/ > insert the new region directly. > > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > --- > mm/memblock.c | 54 +++++++++++++++------------------------------------ > 1 file changed, 16 insertions(+), 38 deletions(-) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 511d4783dcf1..1679244b4a1a 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -578,7 +578,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > phys_addr_t base, phys_addr_t size, > int nid, enum memblock_flags flags) > { > - bool insert = false; > phys_addr_t obase = base; > phys_addr_t end = base + memblock_cap_size(base, &size); > int idx, nr_new; > @@ -598,22 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > return 0; > } > > - /* > - * The worst case is when new range overlaps all existing regions, > - * then we'll need type->cnt + 1 empty regions in @type. So if > - * type->cnt * 2 + 1 is less than type->max, we know > - * that there is enough empty regions in @type, and we can insert > - * regions directly. > - */ > - if (type->cnt * 2 + 1 < type->max) > - insert = true; > - > -repeat: > - /* > - * The following is executed twice. Once with %false @insert and > - * then with %true. The first counts the number of regions needed > - * to accommodate the new area. The second actually inserts them. > - */ > base = obase; > nr_new = 0; I believe nr_new variable is no longer needed, is it? > @@ -635,10 +618,14 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > #endif > WARN_ON(flags != rgn->flags); > nr_new++; > - if (insert) > - memblock_insert_region(type, idx++, base, > - rbase - base, nid, > - flags); > + > + if ((type->cnt >= type->max) && > + (memblock_double_array(type, obase, size) < 0)) if ((type->cnt >= type->max) && memblock_double_array(type, obase, size)) would be just fine. I'd appreciate a comment above the if statement explaining when the allocation is required. > + return -ENOMEM; > + > + memblock_insert_region(type, idx++, base, > + rbase - base, nid, > + flags); > } > /* area below @rend is dealt with, forget about it */ > base = min(rend, end); > @@ -647,28 +634,19 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > /* insert the remaining portion */ > if (base < end) { > nr_new++; > - if (insert) > - memblock_insert_region(type, idx, base, end - base, > - nid, flags); > + if ((type->cnt >= type->max) && > + (memblock_double_array(type, obase, size) < 0)) > + return -ENOMEM; > + > + memblock_insert_region(type, idx, base, end - base, > + nid, flags); > } > > if (!nr_new) > return 0; > > - /* > - * If this was the first round, resize array and repeat for actual > - * insertions; otherwise, merge and return. > - */ > - if (!insert) { > - while (type->cnt + nr_new > type->max) > - if (memblock_double_array(type, obase, size) < 0) > - return -ENOMEM; > - insert = true; > - goto repeat; > - } else { > - memblock_merge_regions(type); > - return 0; > - } > + memblock_merge_regions(type); A blank line here would be nice. > + return 0; > } > > /** > -- > 2.25.1 > >
October 25, 2022 12:15 AM, "Mike Rapoport" <rppt@kernel.org> wrote: > Hi, > > On Wed, Oct 19, 2022 at 08:03:37PM +0800, Yajun Deng wrote: > >> Subject: memblock: remove repeat round > > Please make the patch subject more detailed. Say > > membloc: don't run loop in memblock_add_range() twice > Okay! >> There is no need round twice in memblock_add_range(). >> >> We can call memblock_double_array() to extand the size if type->cnt no > > ^ extend > >> less than type->max before memblock_insert_region(), otherwise we can > > s/no less than/greater or equal to/ > Got it. >> insert the new region directly. >> >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> >> --- >> mm/memblock.c | 54 +++++++++++++++------------------------------------ >> 1 file changed, 16 insertions(+), 38 deletions(-) >> >> diff --git a/mm/memblock.c b/mm/memblock.c >> index 511d4783dcf1..1679244b4a1a 100644 >> --- a/mm/memblock.c >> +++ b/mm/memblock.c >> @@ -578,7 +578,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, >> phys_addr_t base, phys_addr_t size, >> int nid, enum memblock_flags flags) >> { >> - bool insert = false; >> phys_addr_t obase = base; >> phys_addr_t end = base + memblock_cap_size(base, &size); >> int idx, nr_new; >> @@ -598,22 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, >> return 0; >> } >> >> - /* >> - * The worst case is when new range overlaps all existing regions, >> - * then we'll need type->cnt + 1 empty regions in @type. So if >> - * type->cnt * 2 + 1 is less than type->max, we know >> - * that there is enough empty regions in @type, and we can insert >> - * regions directly. >> - */ >> - if (type->cnt * 2 + 1 < type->max) >> - insert = true; >> - >> -repeat: >> - /* >> - * The following is executed twice. Once with %false @insert and >> - * then with %true. The first counts the number of regions needed >> - * to accommodate the new area. The second actually inserts them. >> - */ >> base = obase; >> nr_new = 0; > > I believe nr_new variable is no longer needed, is it? > No, nr_new is needed before memblock_merge_regions() for return. >> @@ -635,10 +618,14 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, >> #endif >> WARN_ON(flags != rgn->flags); >> nr_new++; >> - if (insert) >> - memblock_insert_region(type, idx++, base, >> - rbase - base, nid, >> - flags); >> + >> + if ((type->cnt >= type->max) && >> + (memblock_double_array(type, obase, size) < 0)) > > if ((type->cnt >= type->max) && > memblock_double_array(type, obase, size)) > > would be just fine. > > I'd appreciate a comment above the if statement explaining when the > allocation is required. > Got it. >> + return -ENOMEM; >> + >> + memblock_insert_region(type, idx++, base, >> + rbase - base, nid, >> + flags); >> } >> /* area below @rend is dealt with, forget about it */ >> base = min(rend, end); >> @@ -647,28 +634,19 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, >> /* insert the remaining portion */ >> if (base < end) { >> nr_new++; >> - if (insert) >> - memblock_insert_region(type, idx, base, end - base, >> - nid, flags); >> + if ((type->cnt >= type->max) && >> + (memblock_double_array(type, obase, size) < 0)) >> + return -ENOMEM; >> + >> + memblock_insert_region(type, idx, base, end - base, >> + nid, flags); >> } >> >> if (!nr_new) >> return 0; >> >> - /* >> - * If this was the first round, resize array and repeat for actual >> - * insertions; otherwise, merge and return. >> - */ >> - if (!insert) { >> - while (type->cnt + nr_new > type->max) >> - if (memblock_double_array(type, obase, size) < 0) >> - return -ENOMEM; >> - insert = true; >> - goto repeat; >> - } else { >> - memblock_merge_regions(type); >> - return 0; >> - } >> + memblock_merge_regions(type); > > A blank line here would be nice. > Got it. >> + return 0; >> } >> >> /** >> -- >> 2.25.1 > > -- > Sincerely yours, > Mike.
On Tue, Oct 25, 2022 at 03:24:58AM +0000, Yajun Deng wrote: > October 25, 2022 12:15 AM, "Mike Rapoport" <rppt@kernel.org> wrote: > > >> @@ -598,22 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, > >> return 0; > >> } > >> > >> - /* > >> - * The worst case is when new range overlaps all existing regions, > >> - * then we'll need type->cnt + 1 empty regions in @type. So if > >> - * type->cnt * 2 + 1 is less than type->max, we know > >> - * that there is enough empty regions in @type, and we can insert > >> - * regions directly. > >> - */ > >> - if (type->cnt * 2 + 1 < type->max) > >> - insert = true; > >> - > >> -repeat: > >> - /* > >> - * The following is executed twice. Once with %false @insert and > >> - * then with %true. The first counts the number of regions needed > >> - * to accommodate the new area. The second actually inserts them. > >> - */ > >> base = obase; > >> nr_new = 0; > > > > I believe nr_new variable is no longer needed, is it? > > > No, nr_new is needed before memblock_merge_regions() for return. Why?
October 26, 2022 2:04 PM, "Mike Rapoport" <rppt@kernel.org> wrote: > On Tue, Oct 25, 2022 at 03:24:58AM +0000, Yajun Deng wrote: > >> October 25, 2022 12:15 AM, "Mike Rapoport" <rppt@kernel.org> wrote: >> >> @@ -598,22 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, >> return 0; >> } >> >> - /* >> - * The worst case is when new range overlaps all existing regions, >> - * then we'll need type->cnt + 1 empty regions in @type. So if >> - * type->cnt * 2 + 1 is less than type->max, we know >> - * that there is enough empty regions in @type, and we can insert >> - * regions directly. >> - */ >> - if (type->cnt * 2 + 1 < type->max) >> - insert = true; >> - >> -repeat: >> - /* >> - * The following is executed twice. Once with %false @insert and >> - * then with %true. The first counts the number of regions needed >> - * to accommodate the new area. The second actually inserts them. >> - */ >> base = obase; >> nr_new = 0; >> >> I believe nr_new variable is no longer needed, is it? >> >> No, nr_new is needed before memblock_merge_regions() for return. > > Why? > Sorry, nr_new was removed, and added ocnt variable for the original of type->cnt. I already sent another patch with '[PATCH v2] memblock: don't run loop in memblock_add_range() twice' subject, you can see that. > -- > Sincerely yours, > Mike.
diff --git a/mm/memblock.c b/mm/memblock.c index 511d4783dcf1..1679244b4a1a 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -578,7 +578,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, phys_addr_t base, phys_addr_t size, int nid, enum memblock_flags flags) { - bool insert = false; phys_addr_t obase = base; phys_addr_t end = base + memblock_cap_size(base, &size); int idx, nr_new; @@ -598,22 +597,6 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, return 0; } - /* - * The worst case is when new range overlaps all existing regions, - * then we'll need type->cnt + 1 empty regions in @type. So if - * type->cnt * 2 + 1 is less than type->max, we know - * that there is enough empty regions in @type, and we can insert - * regions directly. - */ - if (type->cnt * 2 + 1 < type->max) - insert = true; - -repeat: - /* - * The following is executed twice. Once with %false @insert and - * then with %true. The first counts the number of regions needed - * to accommodate the new area. The second actually inserts them. - */ base = obase; nr_new = 0; @@ -635,10 +618,14 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, #endif WARN_ON(flags != rgn->flags); nr_new++; - if (insert) - memblock_insert_region(type, idx++, base, - rbase - base, nid, - flags); + + if ((type->cnt >= type->max) && + (memblock_double_array(type, obase, size) < 0)) + return -ENOMEM; + + memblock_insert_region(type, idx++, base, + rbase - base, nid, + flags); } /* area below @rend is dealt with, forget about it */ base = min(rend, end); @@ -647,28 +634,19 @@ static int __init_memblock memblock_add_range(struct memblock_type *type, /* insert the remaining portion */ if (base < end) { nr_new++; - if (insert) - memblock_insert_region(type, idx, base, end - base, - nid, flags); + if ((type->cnt >= type->max) && + (memblock_double_array(type, obase, size) < 0)) + return -ENOMEM; + + memblock_insert_region(type, idx, base, end - base, + nid, flags); } if (!nr_new) return 0; - /* - * If this was the first round, resize array and repeat for actual - * insertions; otherwise, merge and return. - */ - if (!insert) { - while (type->cnt + nr_new > type->max) - if (memblock_double_array(type, obase, size) < 0) - return -ENOMEM; - insert = true; - goto repeat; - } else { - memblock_merge_regions(type); - return 0; - } + memblock_merge_regions(type); + return 0; } /**
There is no need round twice in memblock_add_range(). We can call memblock_double_array() to extand the size if type->cnt no less than type->max before memblock_insert_region(), otherwise we can insert the new region directly. Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- mm/memblock.c | 54 +++++++++++++++------------------------------------ 1 file changed, 16 insertions(+), 38 deletions(-)