+ minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

+ minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[Andrew Morton]

The patch titled
     Subject: minmax.h: simplify the variants of clamp()
has been added to the -mm mm-nonmm-unstable branch.  Its filename is
     minmaxh-simplify-the-variants-of-clamp.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/minmaxh-simplify-the-variants-of-clamp.patch

This patch will later appear in the mm-nonmm-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: David Laight <David.Laight@ACULAB.COM>
Subject: minmax.h: simplify the variants of clamp()
Date: Mon, 18 Nov 2024 19:15:05 +0000

Always pass a 'type' through to __clamp_once(), pass '__auto_type' from
clamp() itself.

The expansion of __types_ok3() is reasonable so it isn't worth the added
complexity of avoiding it when a fixed type is used for all three values.

Link: https://lkml.kernel.org/r/8f69f4deac014f558bab186444bac2e8@AcuMS.aculab.com
Signed-off-by: David Laight <david.laight@aculab.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Mateusz Guzik <mjguzik@gmail.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/minmax.h |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--- a/include/linux/minmax.h~minmaxh-simplify-the-variants-of-clamp
+++ a/include/linux/minmax.h
@@ -183,29 +183,29 @@
 #define __clamp(val, lo, hi)	\
 	((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
 
-#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({				\
-	__auto_type uval = (val);						\
-	__auto_type ulo = (lo);							\
-	__auto_type uhi = (hi);							\
+#define __clamp_once(type, val, lo, hi, uval, ulo, uhi) ({			\
+	type uval = (val);							\
+	type ulo = (lo);							\
+	type uhi = (hi);							\
 	BUILD_BUG_ON_MSG(statically_true(ulo > uhi),				\
 		"clamp() low limit " #lo " greater than high limit " #hi);	\
 	BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi),				\
 		"clamp("#val", "#lo", "#hi") signedness error");		\
 	__clamp(uval, ulo, uhi); })
 
-#define __careful_clamp(val, lo, hi) \
-	__clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
+#define __careful_clamp(type, val, lo, hi) \
+	__clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
 
 /**
- * clamp - return a value clamped to a given range with strict typechecking
+ * clamp - return a value clamped to a given range with typechecking
  * @val: current value
  * @lo: lowest allowable value
  * @hi: highest allowable value
  *
- * This macro does strict typechecking of @lo/@hi to make sure they are of the
- * same type as @val.  See the unnecessary pointer comparisons.
+ * This macro checks @val/@lo/@hi to make sure they have compatible
+ * signedness.
  */
-#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
+#define clamp(val, lo, hi) __careful_clamp(__auto_type, val, lo, hi)
 
 /**
  * clamp_t - return a value clamped to a given range using a given type
@@ -217,7 +217,7 @@
  * This macro does no typechecking and uses temporary variables of type
  * @type to make all the comparisons.
  */
-#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi))
+#define clamp_t(type, val, lo, hi) __careful_clamp(type, val, lo, hi)
 
 /**
  * clamp_val - return a value clamped to a given range using val's type
@@ -230,7 +230,7 @@
  * type and @lo and @hi are literals that will otherwise be assigned a signed
  * integer type.
  */
-#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
+#define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi)
 
 /*
  * Do not check the array parameter using __must_be_array().
_

Patches currently in -mm which might be from David.Laight@ACULAB.COM are

minmaxh-add-whitespace-around-operators-and-after-commas.patch
minmaxh-update-some-comments.patch
minmaxh-reduce-the-define-expansion-of-min-max-and-clamp.patch
minmaxh-use-build_bug_on_msg-for-the-lo-hi-test-in-clamp.patch
minmaxh-move-all-the-clamp-definitions-after-the-min-max-ones.patch
minmaxh-simplify-the-variants-of-clamp.patch
minmaxh-remove-some-defines-that-are-only-expanded-once.patch

Re: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[Lorenzo Stoakes]

On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
>
> The patch titled
>      Subject: minmax.h: simplify the variants of clamp()
> has been added to the -mm mm-nonmm-unstable branch.  Its filename is
>      minmaxh-simplify-the-variants-of-clamp.patch

Hmm, this was generating kernel test bot reports, should we be taking it without
that being fixed?

>
> This patch will shortly appear at
>      https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/minmaxh-simplify-the-variants-of-clamp.patch
>
> This patch will later appear in the mm-nonmm-unstable branch at
>     git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
>
> ------------------------------------------------------
> From: David Laight <David.Laight@ACULAB.COM>
> Subject: minmax.h: simplify the variants of clamp()
> Date: Mon, 18 Nov 2024 19:15:05 +0000
>
> Always pass a 'type' through to __clamp_once(), pass '__auto_type' from
> clamp() itself.
>
> The expansion of __types_ok3() is reasonable so it isn't worth the added
> complexity of avoiding it when a fixed type is used for all three values.
>
> Link: https://lkml.kernel.org/r/8f69f4deac014f558bab186444bac2e8@AcuMS.aculab.com
> Signed-off-by: David Laight <david.laight@aculab.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Arnd Bergmann <arnd@kernel.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Mateusz Guzik <mjguzik@gmail.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  include/linux/minmax.h |   24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> --- a/include/linux/minmax.h~minmaxh-simplify-the-variants-of-clamp
> +++ a/include/linux/minmax.h
> @@ -183,29 +183,29 @@
>  #define __clamp(val, lo, hi)	\
>  	((val) >= (hi) ? (hi) : ((val) <= (lo) ? (lo) : (val)))
>
> -#define __clamp_once(val, lo, hi, uval, ulo, uhi) ({				\
> -	__auto_type uval = (val);						\
> -	__auto_type ulo = (lo);							\
> -	__auto_type uhi = (hi);							\
> +#define __clamp_once(type, val, lo, hi, uval, ulo, uhi) ({			\
> +	type uval = (val);							\
> +	type ulo = (lo);							\
> +	type uhi = (hi);							\
>  	BUILD_BUG_ON_MSG(statically_true(ulo > uhi),				\
>  		"clamp() low limit " #lo " greater than high limit " #hi);	\
>  	BUILD_BUG_ON_MSG(!__types_ok3(uval, ulo, uhi),				\
>  		"clamp("#val", "#lo", "#hi") signedness error");		\
>  	__clamp(uval, ulo, uhi); })
>
> -#define __careful_clamp(val, lo, hi) \
> -	__clamp_once(val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
> +#define __careful_clamp(type, val, lo, hi) \
> +	__clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
>
>  /**
> - * clamp - return a value clamped to a given range with strict typechecking
> + * clamp - return a value clamped to a given range with typechecking
>   * @val: current value
>   * @lo: lowest allowable value
>   * @hi: highest allowable value
>   *
> - * This macro does strict typechecking of @lo/@hi to make sure they are of the
> - * same type as @val.  See the unnecessary pointer comparisons.
> + * This macro checks @val/@lo/@hi to make sure they have compatible
> + * signedness.
>   */
> -#define clamp(val, lo, hi) __careful_clamp(val, lo, hi)
> +#define clamp(val, lo, hi) __careful_clamp(__auto_type, val, lo, hi)
>
>  /**
>   * clamp_t - return a value clamped to a given range using a given type
> @@ -217,7 +217,7 @@
>   * This macro does no typechecking and uses temporary variables of type
>   * @type to make all the comparisons.
>   */
> -#define clamp_t(type, val, lo, hi) __careful_clamp((type)(val), (type)(lo), (type)(hi))
> +#define clamp_t(type, val, lo, hi) __careful_clamp(type, val, lo, hi)
>
>  /**
>   * clamp_val - return a value clamped to a given range using val's type
> @@ -230,7 +230,7 @@
>   * type and @lo and @hi are literals that will otherwise be assigned a signed
>   * integer type.
>   */
> -#define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi)
> +#define clamp_val(val, lo, hi) __careful_clamp(typeof(val), val, lo, hi)
>
>  /*
>   * Do not check the array parameter using __must_be_array().
> _
>
> Patches currently in -mm which might be from David.Laight@ACULAB.COM are
>
> minmaxh-add-whitespace-around-operators-and-after-commas.patch
> minmaxh-update-some-comments.patch
> minmaxh-reduce-the-define-expansion-of-min-max-and-clamp.patch
> minmaxh-use-build_bug_on_msg-for-the-lo-hi-test-in-clamp.patch
> minmaxh-move-all-the-clamp-definitions-after-the-min-max-ones.patch
> minmaxh-simplify-the-variants-of-clamp.patch
> minmaxh-remove-some-defines-that-are-only-expanded-once.patch
>

RE: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[David Laight]

From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Sent: 29 November 2024 11:46
> 
> On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
> >
> > The patch titled
> >      Subject: minmax.h: simplify the variants of clamp()
> > has been added to the -mm mm-nonmm-unstable branch.  Its filename is
> >      minmaxh-simplify-the-variants-of-clamp.patch
> 
> Hmm, this was generating kernel test bot reports, should we be taking it without
> that being fixed?

That is reported for W=1 builds and is correctly reporting some very
badly broken code - that has always been broken.

Probably also only affects allmodconfig builds because the code in
question is unlikely to be included.

	David

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

Re: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[Lorenzo Stoakes]

On Fri, Nov 29, 2024 at 11:56:01AM +0000, David Laight wrote:
> From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Sent: 29 November 2024 11:46
> >
> > On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
> > >
> > > The patch titled
> > >      Subject: minmax.h: simplify the variants of clamp()
> > > has been added to the -mm mm-nonmm-unstable branch.  Its filename is
> > >      minmaxh-simplify-the-variants-of-clamp.patch
> >
> > Hmm, this was generating kernel test bot reports, should we be taking it without
> > that being fixed?
>
> That is reported for W=1 builds and is correctly reporting some very
> badly broken code - that has always been broken.
>
> Probably also only affects allmodconfig builds because the code in
> question is unlikely to be included.

Ah fair play, you are poking around in some creaky, cobweb filled bits of
the kernel so perhaps not a surprise...

We should probably try and sort out the W=1 stuff though! And by 'we' I
mean you obviously ;)

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

RE: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[David Laight]

From: Lorenzo Stoakes
> Sent: 29 November 2024 12:06
> 
> On Fri, Nov 29, 2024 at 11:56:01AM +0000, David Laight wrote:
> > From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > Sent: 29 November 2024 11:46
> > >
> > > On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
> > > >
> > > > The patch titled
> > > >      Subject: minmax.h: simplify the variants of clamp()
> > > > has been added to the -mm mm-nonmm-unstable branch.  Its filename is
> > > >      minmaxh-simplify-the-variants-of-clamp.patch
> > >
> > > Hmm, this was generating kernel test bot reports, should we be taking it without
> > > that being fixed?
> >
> > That is reported for W=1 builds and is correctly reporting some very
> > badly broken code - that has always been broken.
> >
> > Probably also only affects allmodconfig builds because the code in
> > question is unlikely to be included.
> 
> Ah fair play, you are poking around in some creaky, cobweb filled bits of
> the kernel so perhaps not a surprise...
> 
> We should probably try and sort out the W=1 stuff though! And by 'we' I
> mean you obviously ;)

Not me, Jakob is trying to fix the magnetometer/yamaha-yas530.c driver.
But I keep on pointing out just how broken the code was.
So there should be a patch to it (that needs backporting) soon.

	David

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

Re: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[Lorenzo Stoakes]

On Fri, Nov 29, 2024 at 12:10:19PM +0000, David Laight wrote:
> From: Lorenzo Stoakes
> > Sent: 29 November 2024 12:06
> >
> > On Fri, Nov 29, 2024 at 11:56:01AM +0000, David Laight wrote:
> > > From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > Sent: 29 November 2024 11:46
> > > >
> > > > On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
> > > > >
> > > > > The patch titled
> > > > >      Subject: minmax.h: simplify the variants of clamp()
> > > > > has been added to the -mm mm-nonmm-unstable branch.  Its filename is
> > > > >      minmaxh-simplify-the-variants-of-clamp.patch
> > > >
> > > > Hmm, this was generating kernel test bot reports, should we be taking it without
> > > > that being fixed?
> > >
> > > That is reported for W=1 builds and is correctly reporting some very
> > > badly broken code - that has always been broken.
> > >
> > > Probably also only affects allmodconfig builds because the code in
> > > question is unlikely to be included.
> >
> > Ah fair play, you are poking around in some creaky, cobweb filled bits of
> > the kernel so perhaps not a surprise...
> >
> > We should probably try and sort out the W=1 stuff though! And by 'we' I
> > mean you obviously ;)
>
> Not me, Jakob is trying to fix the magnetometer/yamaha-yas530.c driver.
> But I keep on pointing out just how broken the code was.
> So there should be a patch to it (that needs backporting) soon.

Presumably that somehow interacts with this clamp code that he's also
addressing you mean? As that's something of a specific driver (I have no
idea even what a magnetometer is...)

In general your series seems mostly to be various reasonable cleanups (at a
glance) so think we're all good to at least have this in 6.14 unstable
anyway, to be clear.

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

Re: + minmaxh-simplify-the-variants-of-clamp.patch added to mm-nonmm-unstable branch

[Jakob Hauser]

Hi Lorenzo,

On 29.11.24 13:14, Lorenzo Stoakes wrote:
> On Fri, Nov 29, 2024 at 12:10:19PM +0000, David Laight wrote:
>> From: Lorenzo Stoakes
>>> Sent: 29 November 2024 12:06
>>>
>>> On Fri, Nov 29, 2024 at 11:56:01AM +0000, David Laight wrote:
>>>> From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> Sent: 29 November 2024 11:46
>>>>>
>>>>> On Fri, Nov 29, 2024 at 03:11:43AM -0800, Andrew Morton wrote:
>>>>>>
>>>>>> The patch titled
>>>>>>       Subject: minmax.h: simplify the variants of clamp()
>>>>>> has been added to the -mm mm-nonmm-unstable branch.  Its filename is
>>>>>>       minmaxh-simplify-the-variants-of-clamp.patch
>>>>>
>>>>> Hmm, this was generating kernel test bot reports, should we be taking it without
>>>>> that being fixed?
>>>>
>>>> That is reported for W=1 builds and is correctly reporting some very
>>>> badly broken code - that has always been broken.
>>>>
>>>> Probably also only affects allmodconfig builds because the code in
>>>> question is unlikely to be included.
>>>
>>> Ah fair play, you are poking around in some creaky, cobweb filled bits of
>>> the kernel so perhaps not a surprise...
>>>
>>> We should probably try and sort out the W=1 stuff though! And by 'we' I
>>> mean you obviously ;)
>>
>> Not me, Jakob is trying to fix the magnetometer/yamaha-yas530.c driver.
>> But I keep on pointing out just how broken the code was.
>> So there should be a patch to it (that needs backporting) soon.
> 
> Presumably that somehow interacts with this clamp code that he's also
> addressing you mean? As that's something of a specific driver (I have no
> idea even what a magnetometer is...)

I'm working on cleaning up that part in 
drivers/iio/magnetometer/yamaha-yas530.c.
v1: 
https://lore.kernel.org/linux-iio/20241126234021.19749-1-jahau@rocketmail.com/T/#u
v2: 
https://lore.kernel.org/linux-iio/11609b2243c295d65ab4d47e78c239d61ad6be75.1732914810.git.jahau@rocketmail.com/T/#u

> In general your series seems mostly to be various reasonable cleanups (at a
> glance) so think we're all good to at least have this in 6.14 unstable
> anyway, to be clear.

Kind regards,
Jakob