From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C6C5C3DA7F for ; Fri, 26 Jul 2024 12:58:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6D06F6B00A4; Fri, 26 Jul 2024 08:58:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6806A6B00A6; Fri, 26 Jul 2024 08:58:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 547B76B00A8; Fri, 26 Jul 2024 08:58:31 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 36C676B00A4 for ; Fri, 26 Jul 2024 08:58:31 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CA6DA8181E for ; Fri, 26 Jul 2024 12:58:30 +0000 (UTC) X-FDA: 82381907580.04.3577403 Received: from eu-smtp-delivery-151.mimecast.com (eu-smtp-delivery-151.mimecast.com [185.58.85.151]) by imf19.hostedemail.com (Postfix) with ESMTP id 86E5E1A000C for ; Fri, 26 Jul 2024 12:58:27 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of david.laight@aculab.com designates 185.58.85.151 as permitted sender) smtp.mailfrom=david.laight@aculab.com; dmarc=pass (policy=none) header.from=aculab.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721998668; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=kQSU1lhDhLBj6wRkET5kINFcVfR/CaXSYVaZHjwzhsk=; b=AfHRdtjCoAo/tFz5l8sVrrNk7GRikuAKaeAWfspUkdIL5jNRpbTB0skognvyXqGFc3e9qX ADGC+9j/Tixf5XU7xpP8nuf9eYEsDWkoT2/k7rL6HiAoT5NXrtx6/PANUqAkw8UwSJYiSy x0uIagABdSmogfkbRDGwz8NeRu1dDe0= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; spf=pass (imf19.hostedemail.com: domain of david.laight@aculab.com designates 185.58.85.151 as permitted sender) smtp.mailfrom=david.laight@aculab.com; dmarc=pass (policy=none) header.from=aculab.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721998668; a=rsa-sha256; cv=none; b=njOpfBRVf7LEtWDRAz3br5CU8aBCCvs4e+OLeWliGL9o/0jcFxTdsJTOkFzLgfA8jQVl5k riL+umxa4T4K8r8WOd/gqFtS0FViNbHpB6CrMvB5KfjHTOuqzVIxHNWXuhVNqa8IfTI0q4 tRo/ZDpOHfx7TOw4xPbQpb8DluFCjng= Received: from AcuMS.aculab.com (156.67.243.121 [156.67.243.121]) by relay.mimecast.com with ESMTP with both STARTTLS and AUTH (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id uk-mta-138-dErO4UU2Os2Sn7zB_sImjg-1; Fri, 26 Jul 2024 13:58:23 +0100 X-MC-Unique: dErO4UU2Os2Sn7zB_sImjg-1 Received: from AcuMS.Aculab.com (10.202.163.6) by AcuMS.aculab.com (10.202.163.6) with Microsoft SMTP Server (TLS) id 15.0.1497.48; Fri, 26 Jul 2024 13:57:43 +0100 Received: from AcuMS.Aculab.com ([::1]) by AcuMS.aculab.com ([::1]) with mapi id 15.00.1497.048; Fri, 26 Jul 2024 13:57:43 +0100 From: David Laight To: 'Lorenzo Stoakes' , Linus Torvalds CC: Arnd Bergmann , "linux-kernel@vger.kernel.org" , Matthew Wilcox , Christoph Hellwig , Andrew Morton , Andy Shevchenko , Dan Carpenter , "Jason A . Donenfeld" , "pedro.falcato@gmail.com" , Mateusz Guzik , "linux-mm@kvack.org" Subject: RE: [PATCH 4/7] minmax: Simplify signedness check Thread-Topic: [PATCH 4/7] minmax: Simplify signedness check Thread-Index: Adrd1i0k/JcX2h1sSAO9D37F5HIFAAALj27TABqyPTAANETWLQAFsOCg Date: Fri, 26 Jul 2024 12:57:43 +0000 Message-ID: References: <23bdb6fc8d884ceebeb6e8b8653b8cfe@AcuMS.aculab.com> <03601661326c4efba4e618ead15fa0e2@AcuMS.aculab.com> <5a129d04e0b84b48ba6c5189a047ac8f@AcuMS.aculab.com> In-Reply-To: Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.202.205.107] MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: aculab.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Stat-Signature: griim3ckto6y7aiyjcrhxco7shmsanbf X-Rspam-User: X-Rspamd-Queue-Id: 86E5E1A000C X-Rspamd-Server: rspam02 X-HE-Tag: 1721998707-887317 X-HE-Meta: U2FsdGVkX1+UzkAIU5ebKfCqOFZR4u7QWC4eGxSeo/dAiQK5vCaW00/Bo0BAG9ZHY/lJPBrKRVe3Axmo0tV2d/o7vgVXFrEMG5JW58cYr7gjdCLI4SpwcYjblnQ3fUcvVxy66q6D7KvStxDUq1D6H9zhiIRRZGQzOLMa3dnLt/QDtKrGOaUnBc0gc42wOu6fDxL+qtknfkR3aV7TtffbjhtBATb4kXUnoAbQ5gpfee7elwwEmLFN0+/mvtnO2HOstfjZgBzhBt3jmsS5aOBw9cGEFmyGUC8v6KnU1EIAmeGoHJroxbX+vTkmpK8dUuxybiikE2M4CysQ7N/XvGc7RrOP0Uw80Q/6/ccSXhbSsbgpZ2Asq8ARgO/IlW6eq/lj7Ms+5MDDOsnrKPkQk7W9X7BjEOoWfm8p3teg54J+7mu65txxtT1h9KRDSKn88mVIY93TGAGrntQKj+vggGm0E4+Uyt3PjX2mCIo9n+GFnSC4Rxwx7F2DbDyHGMgBg6q0n3Vxbw4aJ4QPDkQJ5h8BflFOCMzHDBdqLmJS8/AtpTkY3RvnJcMHVaMK1OIkjphqiXFlor9dUphUr3gTW6eObP3sOiWTOHOhLQI3k7xqcylNTpmDoKH6eMSajKHULtreT7Bch+ygybWMJoaUYrkaUDnoUN/3fvTVivhNwa2oTy5k03XM0wFQ4scrD+P3rB06/n9Jni2bxxqhrDEKeq1Ek9S3cpAOcvWwVsVtJtwK8KZaQKutdmjTKc6Gu1SNCspCo/tQ5ZSvcMydoqHL16V2z20bJ5m6FHi62AXrf13h54ap4CHoqhbNULcxrz+Ne8k5FfPnqn/1NuGY7aEKzwRFvKUE4VzaGTmnWwQB1EWRNvTalDcwDhjG0VQiJoui3LgTfELx90RdRa2Dk/S6Slv6ZInbB4RbbmbgZnqc4msxYXHipoSfxY0C82jtoyFCkKeX4Jiv5PQbWiG0X1qpYKi Sm21fzUo xWEF4ccT9hmHbbvF5P97UAAKuibV/jA9kOK8Ijr2t0c5SRzeWNMwaSBL9/C4xYI4RUNQwYNPLgkO9yagZVdTZBVgb+dnw4zPIf53tFlp2RxysEm+GvRyZ7i0RQlDxyH+UdrhWed7LYXPnBts5zbtrwxr3f7VTD+xZ8DCUlAhbT3OjRvhuWdEydahEvKcbzm8tX6OgXiLSRXX92rMDfwGdyQ8SpdiRTLbfyFCMxuTdefhd3xcGo4WAt3UJkYM67SL3Eqom+rx/Jtf5vvmbAKYNujORDXmayt1sGCvkTzWNNnjFo2WQxhwtyoIe92XukCXnJbWRTvrB2xXuQsMHvqErm+tNTSXKsel+x55QV0VJWD3Sm59kc1HR2SN1le8I+6NJNsPWF+hbX+Kv6Z6nGp5eM+lVYJ35v4NKShgckDUZKzP8o6REBZN3ZJmyAmVG+tMvG99Z X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: From: Lorenzo Stoakes > Sent: 26 July 2024 10:44 >=20 > On Thu, Jul 25, 2024 at 10:02:45AM GMT, Linus Torvalds wrote: > > On Thu, 25 Jul 2024 at 02:01, David Laight wr= ote: > > > > > > The condition is '>=3D 0' so it doesn't matter if it is '1' or '0'. > > > > Yes, but that's because the whole conditional is so inexplicably comple= x. > > > > But the explanation is: > > > > > That gives a 'comparison of unsigned type against 0 is always true' w= arning. > > > (The compiler generates that for code in the unused branches of both > > > __builtin_choose_expr() and _Generic().) > > > Moving the comparison to the outer level stops all such compiler warn= ings. > > > > Christ. This whole series is a nightmare of "add complexity to deal > > with stupid issues". > > > > But the kernel test robot clearly found even more issues. > > > > I think we need to just go back to the old code. It was stupid and > > limited and caused us to have to be more careful about types than was > > strictly necessary. >=20 > The problem is simply reverting reveals that seemingly a _ton_ of code ha= s > come to rely on the relaxed conditions. >=20 > When I went to gather the numbers for my initial report I had to manually > fix up every case which was rather painful, and that was just a defconfig= + > a few extra options. allmodconfig is likely to be a hellscape. >=20 > I've not dug deep into the ins + outs of this, so forgive me for being > vague (Arnd has a far clearer understanding) - but it seems that the > majority of the complexity comes from having to absolutely ensure all thi= s > works for compile-time constant values. The problems arise due to some odd uses, not just the requirement for compi= le-time constants for on-stack array sizes. The test robot report is for a test between pointers. I thought there was one in the build I do - and it doesn't usually generate= a warning. This one is related to the different between a 'compile time constant' and a 'constant integer expression'. This is due to is_unsigned_type(t) being (t)-1 > 0 but (type *)x not being 'constant enough' unless 'x' is zero. Fixable by something like: =09(__if_constexpr((t)-1, (t)-1, 1) > 0) But that requires two expansions of the type. Now the type could be that of the unique temporary - but that would make it all more complicated. There are fewer min/max of pointers than when constants are needed. So requiring them be min_ptr() wouldn't be a massive change. > Arnd had a look through and determined there weren't _too_ many cases whe= re > we need this (for instance array sizes). >=20 > So I wonder whether we can't just vastly simplify this stuff (and reducin= g > the macro expansion hell) for the usual case, and implement something lik= e > cmin()/cmax() or whatever for the true-constant cases? I did do that in a patch set from much earlier in the year. But Linus said they'd need to be MIN() and MAX() and that requires changes to a few places where those are already defined. > > But it was also about a million times simpler, and didn't cause build > > time regressions. Just bugs because people did min_t(short, 65536, 128) and didn't expect zer= o. It has to be said that the chances of a min(negative_value, unsigned_consta= nt) appearing are pretty slim. All these tests are there to trap that case. There is always the option of disabling the tests for 'normal' build, but leaving them there for (say) the W=3D1 builds. Then it won't matter as much if the tests slow down the build a little. I think I have tried a W=3D1 build - but there are too many warnings/errors from other places to get anywhere. A lot are for 'unsigned_var >=3D 0' in paths that get optimised away. The compiler doesn't help! =09David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1= PT, UK Registration No: 1397386 (Wales)