linux-kernel - RE: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b89b80c7060c43f8a937b8021b4fd7cb@AcuMS.aculab.com>
Date: Sun, 28 Jul 2024 22:13:15 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...uxfoundation.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Jens Axboe
	<axboe@...nel.dk>, "Matthew Wilcox (Oracle)" <willy@...radead.org>,
	"Christoph Hellwig" <hch@...radead.org>, Andrew Morton
	<akpm@...ux-foundation.org>, "Andy Shevchenko"
	<andriy.shevchenko@...ux.intel.com>, Dan Carpenter
	<dan.carpenter@...aro.org>, Arnd Bergmann <arnd@...nel.org>,
	"Jason@...c4.com" <Jason@...c4.com>, "pedro.falcato@...il.com"
	<pedro.falcato@...il.com>, Mateusz Guzik <mjguzik@...il.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, Lorenzo Stoakes
	<lorenzo.stoakes@...cle.com>
Subject: RE: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

From: Linus Torvalds <torvalds@...uxfoundation.org>
> Sent: 28 July 2024 21:32
> 
> On Sun, 28 Jul 2024 at 13:23, David Laight <David.Laight@...lab.com> wrote:
> >
> > MIN() (and probably your MIN_T()) ought to have a check for
> > being a constant in order to stop misuse.
> 
> No, we have a number of "runtime constants" that are basically
> "constants" set up at boot-time for the architecture,as pointed out by
> the powerpc people in private:
> 
> Ie, we have arch/powerpc/include/asm/page.h:
> 
>    #define HPAGE_SHIFT hpage_shift
> 
> and then
> 
>   #define HUGETLB_PAGE_ORDER      (HPAGE_SHIFT - PAGE_SHIFT)
> 
> and then
> 
>    #define pageblock_order         MIN_T(unsigned int,
> HUGETLB_PAGE_ORDER, MAX_PAGE_ORDER)
> 
> and we really *REALLY* don't want to force the complicated "min_t()"
> (or, worse yet, "min()") functions here just because there's actually
> a variable involved.
> 
> That variable gets initialized early in
> hugetlbpage_init_defaultsize(), so it's *effectively* a constant, but
> not as far as the compiler is concerned.

Ok, but those can't be used as array sizes or constants.
So the temporaries don't matter.
Don't they just work with min() - if not where is the signednes mismatch?

Perhaps we want:
	min() - temporaries, signedness check.
	min_t() - temporaries of given type, maybe check size not reduced?
	MIN() - no temporaries, no signedness check, only valid for constants.
	_min() - temporaries, no signedness check.
	_MIN() - no temporaries, no signedness check, variables allowed.

I'm not sure where your MIN_T() fits in the above.

Personally I think min_t() was a mistake.
Only one input can need a cast and an explicit cast would be safer.

	David

> 
>            Linus

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