Messages in this thread | | | Date | Thu, 6 Apr 2023 06:57:41 -0700 | Subject | Re: [PATCH] mm-treewide-redefine-max_order-sanely-fix.txt | From | Guenter Roeck <> |
| |
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
| |