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@xxxxxxxxx> > > Link: https://lore.kernel.org/oe-kbuild-all/202303152343.D93IbJmn-lkp@xxxxxxxxx/ > > Signed-off-by: "Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> > > Cc: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx> > > Cc: Zi Yan <ziy@xxxxxxxxxx> > > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > 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
