* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt @ 2023-04-06 5:20 Guenter Roeck 2023-04-06 7:25 ` Kirill A. Shutemov 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2023-04-06 5:20 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, linux-mm, linux-kernel Hi, On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: > fix min() warning > > Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box > Reported-by: kernel test robot <lkp@intel.com> > Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ > Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Cc: Zi Yan <ziy@nvidia.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> This patch results in various boot failures (hang) on arm targets in linux-next. Debug messages reveal the reason. ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() interprets as such, while min() apparently uses the returned unsigned long value. Obviously a negative order isn't received well by the rest of the code. Guenter > --- > mm/memblock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 338b8cb0793e..7911224b1ed3 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -2043,7 +2043,7 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) > int order; > > while (start < end) { > - order = min(MAX_ORDER, __ffs(start)); > + order = min_t(int, MAX_ORDER, __ffs(start)); > > while (start + (1UL << order) > end) > order--; > -- > 2.39.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 5:20 [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt Guenter Roeck @ 2023-04-06 7:25 ` Kirill A. Shutemov 2023-04-06 13:57 ` Guenter Roeck 0 siblings, 1 reply; 9+ messages in thread From: Kirill A. Shutemov @ 2023-04-06 7:25 UTC (permalink / raw) To: Guenter Roeck; +Cc: Andrew Morton, linux-mm, linux-kernel On Wed, Apr 05, 2023 at 10:20:26PM -0700, Guenter Roeck wrote: > Hi, > > On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: > > fix min() warning > > > > Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box > > Reported-by: kernel test robot <lkp@intel.com> > > Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ > > Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > Cc: Zi Yan <ziy@nvidia.com> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > This patch results in various boot failures (hang) on arm targets > in linux-next. Debug messages reveal the reason. > > ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() > interprets as such, while min() apparently uses the returned unsigned long > value. Obviously a negative order isn't received well by the rest of the > code. Actually, __ffs() is not defined for 0. Maybe something like this? diff --git a/mm/memblock.c b/mm/memblock.c index 7911224b1ed3..63603b943bd0 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -2043,7 +2043,11 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start < end) { - order = min_t(int, MAX_ORDER, __ffs(start)); + /* __ffs() behaviour is undefined for 0 */ + if (start) + order = min_t(int, MAX_ORDER, __ffs(start)); + else + order = MAX_ORDER; while (start + (1UL << order) > end) order--; diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index c8f0a8c2d049..3179c30f7958 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -605,7 +605,13 @@ static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages) * this and the first chunk to online will be pageblock_nr_pages. */ for (pfn = start_pfn; pfn < end_pfn;) { - int order = min_t(int, MAX_ORDER, __ffs(pfn)); + int order; + + /* __ffs() behaviour is undefined for 0 */ + if (pfn) + order = min_t(int, MAX_ORDER, __ffs(pfn)); + else + order = MAX_ORDER; (*online_page_callback)(pfn_to_page(pfn), order); pfn += (1UL << order); -- Kiryl Shutsemau / Kirill A. Shutemov ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 7:25 ` Kirill A. Shutemov @ 2023-04-06 13:57 ` Guenter Roeck 2023-04-06 15:10 ` Kirill A. Shutemov 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2023-04-06 13:57 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, linux-mm, linux-kernel On 4/6/23 00:25, Kirill A. Shutemov wrote: > On Wed, Apr 05, 2023 at 10:20:26PM -0700, Guenter Roeck wrote: >> Hi, >> >> On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: >>> fix min() warning >>> >>> Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box >>> Reported-by: kernel test robot <lkp@intel.com> >>> Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ >>> Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> >>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> >>> Cc: Zi Yan <ziy@nvidia.com> >>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> >> >> This patch results in various boot failures (hang) on arm targets >> in linux-next. Debug messages reveal the reason. >> >> ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() >> interprets as such, while min() apparently uses the returned unsigned long >> value. Obviously a negative order isn't received well by the rest of the >> code. > > Actually, __ffs() is not defined for 0. > > Maybe something like this? > > diff --git a/mm/memblock.c b/mm/memblock.c > index 7911224b1ed3..63603b943bd0 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -2043,7 +2043,11 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) > int order; > > while (start < end) { > - order = min_t(int, MAX_ORDER, __ffs(start)); > + /* __ffs() behaviour is undefined for 0 */ > + if (start) > + order = min_t(int, MAX_ORDER, __ffs(start)); > + else > + order = MAX_ORDER; > Shouldn't that be else order = 0; ? Guenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 13:57 ` Guenter Roeck @ 2023-04-06 15:10 ` Kirill A. Shutemov 2023-04-06 18:23 ` Guenter Roeck 2023-04-06 21:14 ` Mike Rapoport 0 siblings, 2 replies; 9+ messages in thread From: Kirill A. Shutemov @ 2023-04-06 15:10 UTC (permalink / raw) To: Guenter Roeck, Mike Rapoport; +Cc: Andrew Morton, linux-mm, linux-kernel On Thu, Apr 06, 2023 at 06:57:41AM -0700, Guenter Roeck wrote: > On 4/6/23 00:25, Kirill A. Shutemov wrote: > > On Wed, Apr 05, 2023 at 10:20:26PM -0700, Guenter Roeck wrote: > > > Hi, > > > > > > On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: > > > > fix min() warning > > > > > > > > Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ > > > > Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> > > > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > > > Cc: Zi Yan <ziy@nvidia.com> > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > > > > > This patch results in various boot failures (hang) on arm targets > > > in linux-next. Debug messages reveal the reason. > > > > > > ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > > If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() > > > interprets as such, while min() apparently uses the returned unsigned long > > > value. Obviously a negative order isn't received well by the rest of the > > > code. > > > > Actually, __ffs() is not defined for 0. > > > > Maybe something like this? > > > > diff --git a/mm/memblock.c b/mm/memblock.c > > index 7911224b1ed3..63603b943bd0 100644 > > --- a/mm/memblock.c > > +++ b/mm/memblock.c > > @@ -2043,7 +2043,11 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) > > int order; > > while (start < end) { > > - order = min_t(int, MAX_ORDER, __ffs(start)); > > + /* __ffs() behaviour is undefined for 0 */ > > + if (start) > > + order = min_t(int, MAX_ORDER, __ffs(start)); > > + else > > + order = MAX_ORDER; > > Shouldn't that be > else > order = 0; > ? +Mike. No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the largest chunks alignment allows. -- Kiryl Shutsemau / Kirill A. Shutemov ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 15:10 ` Kirill A. Shutemov @ 2023-04-06 18:23 ` Guenter Roeck 2023-04-06 21:14 ` Mike Rapoport 1 sibling, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2023-04-06 18:23 UTC (permalink / raw) To: Kirill A. Shutemov, Mike Rapoport; +Cc: Andrew Morton, linux-mm, linux-kernel On 4/6/23 08:10, Kirill A. Shutemov wrote: > On Thu, Apr 06, 2023 at 06:57:41AM -0700, Guenter Roeck wrote: >> On 4/6/23 00:25, Kirill A. Shutemov wrote: >>> On Wed, Apr 05, 2023 at 10:20:26PM -0700, Guenter Roeck wrote: >>>> Hi, >>>> >>>> On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: >>>>> fix min() warning >>>>> >>>>> Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box >>>>> Reported-by: kernel test robot <lkp@intel.com> >>>>> Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ >>>>> Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> >>>>> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> >>>>> Cc: Zi Yan <ziy@nvidia.com> >>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> >>>> >>>> This patch results in various boot failures (hang) on arm targets >>>> in linux-next. Debug messages reveal the reason. >>>> >>>> ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 >>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >>>> >>>> If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() >>>> interprets as such, while min() apparently uses the returned unsigned long >>>> value. Obviously a negative order isn't received well by the rest of the >>>> code. >>> >>> Actually, __ffs() is not defined for 0. >>> >>> Maybe something like this? >>> >>> diff --git a/mm/memblock.c b/mm/memblock.c >>> index 7911224b1ed3..63603b943bd0 100644 >>> --- a/mm/memblock.c >>> +++ b/mm/memblock.c >>> @@ -2043,7 +2043,11 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) >>> int order; >>> while (start < end) { >>> - order = min_t(int, MAX_ORDER, __ffs(start)); >>> + /* __ffs() behaviour is undefined for 0 */ >>> + if (start) >>> + order = min_t(int, MAX_ORDER, __ffs(start)); >>> + else >>> + order = MAX_ORDER; >> >> Shouldn't that be >> else >> order = 0; >> ? > > +Mike. > > No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the > largest chunks alignment allows. > Ah, ok. Makes sense. Thanks, Guenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 15:10 ` Kirill A. Shutemov 2023-04-06 18:23 ` Guenter Roeck @ 2023-04-06 21:14 ` Mike Rapoport 2023-04-06 22:44 ` Andrew Morton 1 sibling, 1 reply; 9+ messages in thread From: Mike Rapoport @ 2023-04-06 21:14 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Guenter Roeck, Andrew Morton, linux-mm, linux-kernel On Thu, Apr 06, 2023 at 06:10:15PM +0300, Kirill A. Shutemov wrote: > On Thu, Apr 06, 2023 at 06:57:41AM -0700, Guenter Roeck wrote: > > On 4/6/23 00:25, Kirill A. Shutemov wrote: > > > On Wed, Apr 05, 2023 at 10:20:26PM -0700, Guenter Roeck wrote: > > > > Hi, > > > > > > > > On Wed, Mar 15, 2023 at 06:38:00PM +0300, Kirill A. Shutemov wrote: > > > > > fix min() warning > > > > > > > > > > Link: https://lkml.kernel.org/r/20230315153800.32wib3n5rickolvh@box > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@intel.com/ > > > > > Signed-off-by: "Kirill A. Shutemov" <kirill@shutemov.name> > > > > > Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > > > > Cc: Zi Yan <ziy@nvidia.com> > > > > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > > > > > > > This patch results in various boot failures (hang) on arm targets > > > > in linux-next. Debug messages reveal the reason. > > > > > > > > ########### MAX_ORDER=10 start=0 __ffs(start)=-1 min()=10 min_t=-1 > > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > > > > > If start==0, __ffs(start) returns 0xfffffff or (as int) -1, which min_t() > > > > interprets as such, while min() apparently uses the returned unsigned long > > > > value. Obviously a negative order isn't received well by the rest of the > > > > code. > > > > > > Actually, __ffs() is not defined for 0. > > > > > > Maybe something like this? > > > > > > diff --git a/mm/memblock.c b/mm/memblock.c > > > index 7911224b1ed3..63603b943bd0 100644 > > > --- a/mm/memblock.c > > > +++ b/mm/memblock.c > > > @@ -2043,7 +2043,11 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) > > > int order; > > > while (start < end) { > > > - order = min_t(int, MAX_ORDER, __ffs(start)); > > > + /* __ffs() behaviour is undefined for 0 */ > > > + if (start) > > > + order = min_t(int, MAX_ORDER, __ffs(start)); > > > + else > > > + order = MAX_ORDER; > > > > Shouldn't that be > > else > > order = 0; > > ? > > +Mike. > > No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the > largest chunks alignment allows. Right. Before the changes to MAX_ORDER it was order = min(MAX_ORDER - 1UL, __ffs(start)); which would evaluate to 10. I'd just prefer the comment to include the explanation about why we choose MAX_ORDER for start == 0. Say /* * __ffs() behaviour is undefined for 0 and we want to free the * pages in the largest chunks alignment allows, so set order to * MAX_ORDER when start == 0 */ > -- > Kiryl Shutsemau / Kirill A. Shutemov -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 21:14 ` Mike Rapoport @ 2023-04-06 22:44 ` Andrew Morton 2023-04-07 12:40 ` Kirill A. Shutemov 0 siblings, 1 reply; 9+ messages in thread From: Andrew Morton @ 2023-04-06 22:44 UTC (permalink / raw) To: Mike Rapoport; +Cc: Kirill A. Shutemov, Guenter Roeck, linux-mm, linux-kernel On Fri, 7 Apr 2023 00:14:31 +0300 Mike Rapoport <rppt@kernel.org> wrote: > > > Shouldn't that be > > > else > > > order = 0; > > > ? > > > > +Mike. > > > > No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the > > largest chunks alignment allows. > > Right. Before the changes to MAX_ORDER it was > > order = min(MAX_ORDER - 1UL, __ffs(start)); > > which would evaluate to 10. > > I'd just prefer the comment to include the explanation about why we choose > MAX_ORDER for start == 0. Say > > /* > * __ffs() behaviour is undefined for 0 and we want to free the > * pages in the largest chunks alignment allows, so set order to > * MAX_ORDER when start == 0 > */ Meanwhile I'd like to fix "various boot failures (hang) on arm targets" in -next, so I queued up Kirill's informal fix for now. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-06 22:44 ` Andrew Morton @ 2023-04-07 12:40 ` Kirill A. Shutemov 2023-04-07 18:03 ` Mike Rapoport 0 siblings, 1 reply; 9+ messages in thread From: Kirill A. Shutemov @ 2023-04-07 12:40 UTC (permalink / raw) To: Andrew Morton; +Cc: Mike Rapoport, Guenter Roeck, linux-mm, linux-kernel On Thu, Apr 06, 2023 at 03:44:23PM -0700, Andrew Morton wrote: > On Fri, 7 Apr 2023 00:14:31 +0300 Mike Rapoport <rppt@kernel.org> wrote: > > > > > Shouldn't that be > > > > else > > > > order = 0; > > > > ? > > > > > > +Mike. > > > > > > No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the > > > largest chunks alignment allows. > > > > Right. Before the changes to MAX_ORDER it was > > > > order = min(MAX_ORDER - 1UL, __ffs(start)); > > > > which would evaluate to 10. > > > > I'd just prefer the comment to include the explanation about why we choose > > MAX_ORDER for start == 0. Say > > > > /* > > * __ffs() behaviour is undefined for 0 and we want to free the > > * pages in the largest chunks alignment allows, so set order to > > * MAX_ORDER when start == 0 > > */ > > Meanwhile I'd like to fix "various boot failures (hang) on arm targets" > in -next, so I queued up Kirill's informal fix for now. Here's my variant of the fix up with more vervose comments. diff --git a/mm/memblock.c b/mm/memblock.c index 7911224b1ed3..381e36ac9e4d 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -2043,7 +2043,16 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) int order; while (start < end) { - order = min_t(int, MAX_ORDER, __ffs(start)); + /* + * Free the pages in the largest chunks alignment allows. + * + * __ffs() behaviour is undefined for 0. start == 0 is + * MAX_ORDER-aligned, Set order to MAX_ORDER for the case. + */ + if (start) + order = min_t(int, MAX_ORDER, __ffs(start)); + else + order = MAX_ORDER; while (start + (1UL << order) > end) order--; diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index c8f0a8c2d049..8e0fa209d533 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -605,7 +605,18 @@ static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages) * this and the first chunk to online will be pageblock_nr_pages. */ for (pfn = start_pfn; pfn < end_pfn;) { - int order = min_t(int, MAX_ORDER, __ffs(pfn)); + int order; + + /* + * Free to online pages in the largest chunks alignment allows. + * + * __ffs() behaviour is undefined for 0. start == 0 is + * MAX_ORDER-aligned, Set order to MAX_ORDER for the case. + */ + if (pfn) + order = min_t(int, MAX_ORDER, __ffs(pfn)); + else + order = MAX_ORDER; (*online_page_callback)(pfn_to_page(pfn), order); pfn += (1UL << order); -- Kiryl Shutsemau / Kirill A. Shutemov ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt 2023-04-07 12:40 ` Kirill A. Shutemov @ 2023-04-07 18:03 ` Mike Rapoport 0 siblings, 0 replies; 9+ messages in thread From: Mike Rapoport @ 2023-04-07 18:03 UTC (permalink / raw) To: Kirill A. Shutemov; +Cc: Andrew Morton, Guenter Roeck, linux-mm, linux-kernel On Fri, Apr 07, 2023 at 03:40:54PM +0300, Kirill A. Shutemov wrote: > On Thu, Apr 06, 2023 at 03:44:23PM -0700, Andrew Morton wrote: > > On Fri, 7 Apr 2023 00:14:31 +0300 Mike Rapoport <rppt@kernel.org> wrote: > > > > > > > Shouldn't that be > > > > > else > > > > > order = 0; > > > > > ? > > > > > > > > +Mike. > > > > > > > > No. start == 0 is MAX_ORDER-aligned. We want to free the pages in the > > > > largest chunks alignment allows. > > > > > > Right. Before the changes to MAX_ORDER it was > > > > > > order = min(MAX_ORDER - 1UL, __ffs(start)); > > > > > > which would evaluate to 10. > > > > > > I'd just prefer the comment to include the explanation about why we choose > > > MAX_ORDER for start == 0. Say > > > > > > /* > > > * __ffs() behaviour is undefined for 0 and we want to free the > > > * pages in the largest chunks alignment allows, so set order to > > > * MAX_ORDER when start == 0 > > > */ > > > > Meanwhile I'd like to fix "various boot failures (hang) on arm targets" > > in -next, so I queued up Kirill's informal fix for now. > > Here's my variant of the fix up with more vervose comments. > > diff --git a/mm/memblock.c b/mm/memblock.c > index 7911224b1ed3..381e36ac9e4d 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -2043,7 +2043,16 @@ static void __init __free_pages_memory(unsigned long start, unsigned long end) > int order; > > while (start < end) { > - order = min_t(int, MAX_ORDER, __ffs(start)); > + /* > + * Free the pages in the largest chunks alignment allows. > + * > + * __ffs() behaviour is undefined for 0. start == 0 is > + * MAX_ORDER-aligned, Set order to MAX_ORDER for the case. ^ small s otherwise feel free to add Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org> > + */ > + if (start) > + order = min_t(int, MAX_ORDER, __ffs(start)); > + else > + order = MAX_ORDER; > > while (start + (1UL << order) > end) > order--; > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index c8f0a8c2d049..8e0fa209d533 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -605,7 +605,18 @@ static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages) > * this and the first chunk to online will be pageblock_nr_pages. > */ > for (pfn = start_pfn; pfn < end_pfn;) { > - int order = min_t(int, MAX_ORDER, __ffs(pfn)); > + int order; > + > + /* > + * Free to online pages in the largest chunks alignment allows. > + * > + * __ffs() behaviour is undefined for 0. start == 0 is > + * MAX_ORDER-aligned, Set order to MAX_ORDER for the case. > + */ > + if (pfn) > + order = min_t(int, MAX_ORDER, __ffs(pfn)); > + else > + order = MAX_ORDER; > > (*online_page_callback)(pfn_to_page(pfn), order); > pfn += (1UL << order); > -- > Kiryl Shutsemau / Kirill A. Shutemov -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-07 18:13 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-06 5:20 [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt Guenter Roeck 2023-04-06 7:25 ` Kirill A. Shutemov 2023-04-06 13:57 ` Guenter Roeck 2023-04-06 15:10 ` Kirill A. Shutemov 2023-04-06 18:23 ` Guenter Roeck 2023-04-06 21:14 ` Mike Rapoport 2023-04-06 22:44 ` Andrew Morton 2023-04-07 12:40 ` Kirill A. Shutemov 2023-04-07 18:03 ` Mike Rapoport
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox