Re: [PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions

2024-02-26 Thread kernel test robot 
Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linux/master mkl-can-next/testing kdave/for-next 
akpm-mm/mm-nonmm-unstable linus/master v6.8-rc6]
[cannot apply to next-20240223 dtor-input/next dtor-input/for-linus 
axboe-block/for-next horms-ipvs/master next-20240223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-definitions-together/20240226-005902
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/a18dcae310f74dcb9c6fc01d5bdc0568%40AcuMS.aculab.com
patch subject: [PATCH next v2 11/11] minmax: min() and max() don't need to 
return constant expressions
config: i386-randconfig-011-20240226 
(https://download.01.org/0day-ci/archive/20240226/202402261802.9shoxrwy-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240226/202402261802.9shoxrwy-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202402261802.9shoxrwy-...@intel.com/

All warnings (new ones prefixed by >>):

   arch/x86/mm/pgtable.c: In function 'pgd_alloc':
>> arch/x86/mm/pgtable.c:437:9: warning: ISO C90 forbids variable length array 
>> 'pmds' [-Wvla]
 437 | pmd_t *pmds[MAX_PREALLOCATED_PMDS];
 | ^


vim +/pmds +437 arch/x86/mm/pgtable.c

1db491f77b6ed0 Fenghua Yu  2015-01-15  432  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  433  pgd_t *pgd_alloc(struct 
mm_struct *mm)
1ec1fe73dfb711 Ingo Molnar 2008-03-19  434  {
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  435  pgd_t *pgd;
184d47f0fd3651 Kees Cook   2018-10-08  436  pmd_t 
*u_pmds[MAX_PREALLOCATED_USER_PMDS];
184d47f0fd3651 Kees Cook   2018-10-08 @437  pmd_t 
*pmds[MAX_PREALLOCATED_PMDS];
1ec1fe73dfb711 Ingo Molnar 2008-03-19  438  
1db491f77b6ed0 Fenghua Yu  2015-01-15  439  pgd = _pgd_alloc();
1ec1fe73dfb711 Ingo Molnar 2008-03-19  440  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  441  if (pgd == NULL)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  442  goto out;
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  443  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  444  mm->pgd = pgd;
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  445  
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  446  if (sizeof(pmds) != 0 &&
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  447  
preallocate_pmds(mm, pmds, PREALLOCATED_PMDS) != 0)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  448  goto 
out_free_pgd;
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  449  
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  450  if (sizeof(u_pmds) != 0 
&&
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  451  
preallocate_pmds(mm, u_pmds, PREALLOCATED_USER_PMDS) != 0)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  452  goto 
out_free_pmds;
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  453  
f59dbe9ca6707e Joerg Roedel2018-07-18  454  if 
(paravirt_pgd_alloc(mm) != 0)
f59dbe9ca6707e Joerg Roedel2018-07-18  455  goto 
out_free_user_pmds;
f59dbe9ca6707e Joerg Roedel2018-07-18  456  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  457  /*
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  458   * Make sure that 
pre-populating the pmds is atomic with
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  459   * respect to anything 
walking the pgd_list, so that they
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  460   * never see a 
partially populated pgd.
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  461   */
a79e53d85683c6 Andrea Arcangeli2011-02-16  462  spin_lock(_lock);
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  463  
617d34d9e5d832 Jeremy Fitzhardinge 2010-09-21  464  pgd_ctor(mm, pgd);
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  465  if (sizeof(pmds) != 0)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  466  
pgd_prepopulate_pmd(mm, pgd, pmds);
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  467  
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  468  if (sizeof(u_pmds) != 0)
f59dbe9ca6707e Joerg Roedel2018-07-18  469  
pgd_prepopulate_user_pmd(mm, pgd, u_pmds);
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  470  
a79e53d85683c6 Andrea Arcangeli2011-02-16  471  spin_unlock(_lock);
4f76cd382213b

RE: [PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions

2024-02-26 Thread David Laight 
From: kernel test robot 
> Sent: 26 February 2024 09:42

> If you fix the issue in a separate patch/commit (i.e. not just a new version 
> of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot 
> | Closes: 
> https://lore.kernel.org/oe-kbuild-all/202402261720.eamc0ehm-...@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> arch/x86/mm/pgtable.c:437:14: warning: variable length array used [-Wvla]
>  437 | pmd_t *pmds[MAX_PREALLOCATED_PMDS];

Not surprisingly I missed X86_CONFIG_PAE builds :-)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)

Re: [PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions

2024-02-26 Thread kernel test robot 
Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linux/master mkl-can-next/testing kdave/for-next 
akpm-mm/mm-nonmm-unstable linus/master v6.8-rc6]
[cannot apply to next-20240223 dtor-input/next dtor-input/for-linus 
axboe-block/for-next horms-ipvs/master next-20240223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/David-Laight/minmax-Put-all-the-clamp-definitions-together/20240226-005902
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/a18dcae310f74dcb9c6fc01d5bdc0568%40AcuMS.aculab.com
patch subject: [PATCH next v2 11/11] minmax: min() and max() don't need to 
return constant expressions
config: i386-buildonly-randconfig-001-20240226 
(https://download.01.org/0day-ci/archive/20240226/202402261720.eamc0ehm-...@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 
6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240226/202402261720.eamc0ehm-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202402261720.eamc0ehm-...@intel.com/

All warnings (new ones prefixed by >>):

>> arch/x86/mm/pgtable.c:437:14: warning: variable length array used [-Wvla]
 437 | pmd_t *pmds[MAX_PREALLOCATED_PMDS];
 | ^
   arch/x86/mm/pgtable.c:180:31: note: expanded from macro 
'MAX_PREALLOCATED_PMDS'
 180 | #define MAX_PREALLOCATED_PMDS   MAX_UNSHARED_PTRS_PER_PGD
 | ^
   arch/x86/mm/pgtable.c:113:2: note: expanded from macro 
'MAX_UNSHARED_PTRS_PER_PGD'
 113 | max_t(size_t, KERNEL_PGD_BOUNDARY, PTRS_PER_PGD)
 | ^~~~
   include/linux/minmax.h:152:27: note: expanded from macro 'max_t'
 152 | #define max_t(type, x, y)   __careful_cmp(max, (type)(x), 
(type)(y), __COUNTER__)
 | 
^
   include/linux/minmax.h:62:2: note: expanded from macro '__careful_cmp'
  59 | #define __careful_cmp(op, x, y, uniq) ({\
 |   ~~~
  60 | _Static_assert(__types_ok(x, y),\
 | ~
  61 | #op "(" #x ", " #y ") signedness error, fix types or 
consider u" #op "() before " #op "_t()"); \
 | 

  62 | __cmp_once(op, x, y, uniq); })
 | ^~
   include/linux/minmax.h:57:2: note: expanded from macro '__cmp_once'
  57 | __cmp(op, __x_##uniq, __y_##uniq); })
 | ^
   include/linux/minmax.h:52:26: note: expanded from macro '__cmp'
  52 | #define __cmp(op, x, y) ((x) __cmp_op_##op (y) ? (x) : (y))
 |  ^
   1 warning generated.


vim +437 arch/x86/mm/pgtable.c

1db491f77b6ed0 Fenghua Yu  2015-01-15  432  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  433  pgd_t *pgd_alloc(struct 
mm_struct *mm)
1ec1fe73dfb711 Ingo Molnar 2008-03-19  434  {
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  435  pgd_t *pgd;
184d47f0fd3651 Kees Cook   2018-10-08  436  pmd_t 
*u_pmds[MAX_PREALLOCATED_USER_PMDS];
184d47f0fd3651 Kees Cook   2018-10-08 @437  pmd_t 
*pmds[MAX_PREALLOCATED_PMDS];
1ec1fe73dfb711 Ingo Molnar 2008-03-19  438  
1db491f77b6ed0 Fenghua Yu  2015-01-15  439  pgd = _pgd_alloc();
1ec1fe73dfb711 Ingo Molnar 2008-03-19  440  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  441  if (pgd == NULL)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  442  goto out;
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  443  
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  444  mm->pgd = pgd;
4f76cd382213b2 Jeremy Fitzhardinge 2008-03-17  445  
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  446  if (sizeof(pmds) != 0 &&
25226df4b9be7f Gustavo A. R. Silva 2022-09-21  447  
preallocate_pmds(mm, pmds, PREALLOCATED_PMDS) != 0)
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  448  goto 
out_free_pgd;
d8d5900ef8afc5 Jeremy Fitzhardinge 2008-06-25  449  
25226d

[PATCH next v2 11/11] minmax: min() and max() don't need to return constant expressions

2024-02-25 Thread David Laight 
After changing the handful of places max() was used to size an on-stack
array to use max_const() it is no longer necessary for min() and max()
to return constant expressions from constant inputs.
Remove the associated logic to reduce the expanded text.

Remove the 'hack' that allowed max(bool, bool).

Fixup the initial block comment to match current reality.

Signed-off-by: David Laight 
---
 include/linux/minmax.h | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

Changes for v2:
- Typographical and spelling corrections to the commit messages.
  Patches unchanged.

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index c08916588425..5e65c98ff256 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -8,13 +8,10 @@
 #include 
 
 /*
- * min()/max()/clamp() macros must accomplish three things:
+ * min()/max()/clamp() macros must accomplish several things:
  *
  * - Avoid multiple evaluations of the arguments (so side-effects like
  *   "x++" happen only once) when non-constant.
- * - Retain result as a constant expressions when called with only
- *   constant expressions (to avoid tripping VLA warnings in stack
- *   allocation usage).
  * - Perform signed v unsigned type-checking (to generate compile
  *   errors instead of nasty runtime surprises).
  * - Unsigned char/short are always promoted to signed int and can be
@@ -22,13 +19,19 @@
  * - Unsigned arguments can be compared against non-negative signed constants.
  * - Comparison of a signed argument against an unsigned constant fails
  *   even if the constant is below __INT_MAX__ and could be cast to int.
+ *
+ * The return value of min()/max() is not a constant expression for
+ * constant parameters - so will trigger a VLA warging if used to size
+ * an on-stack array.
+ * Instead use min_const() or max_const() which do generate constant
+ * expressions and are also valid for static initialisers.
  */
 #define __typecheck(x, y) \
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
 
 /* Allow unsigned compares against non-negative signed constants. */
 #define __is_ok_unsigned(x) \
-   (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) + 0 >= 0 : 0))
+   (is_unsigned_type(typeof(x)) || (__is_constexpr(x) ? (x) >= 0 : 0))
 
 /* Check for signed after promoting unsigned char/short to int */
 #define __is_ok_signed(x) is_signed_type(typeof((x) + 0))
@@ -53,12 +56,10 @@
typeof(y) __y_##uniq = (y); \
__cmp(op, __x_##uniq, __y_##uniq); })
 
-#define __careful_cmp(op, x, y, uniq)  \
-   __builtin_choose_expr(__is_constexpr((x) - (y)),\
-   __cmp(op, x, y),\
-   ({ _Static_assert(__types_ok(x, y), \
-   #op "(" #x ", " #y ") signedness error, fix types or 
consider u" #op "() before " #op "_t()"); \
-   __cmp_once(op, x, y, uniq); }))
+#define __careful_cmp(op, x, y, uniq) ({   \
+   _Static_assert(__types_ok(x, y),\
+   #op "(" #x ", " #y ") signedness error, fix types or consider 
u" #op "() before " #op "_t()"); \
+   __cmp_once(op, x, y, uniq); })
 
 #define __careful_cmp_const(op, x, y)  \
(BUILD_BUG_ON_ZERO(!__is_constexpr((x) - (y))) +\
-- 
2.17.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)