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 C3006C3DA4A for ; Mon, 29 Jul 2024 22:25:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3A12D6B0085; Mon, 29 Jul 2024 18:25:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 329F26B0088; Mon, 29 Jul 2024 18:25:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1A4B76B0089; Mon, 29 Jul 2024 18:25:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id EC6DD6B0085 for ; Mon, 29 Jul 2024 18:25:53 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 77C39A0117 for ; Mon, 29 Jul 2024 22:25:53 +0000 (UTC) X-FDA: 82394223786.05.0B114D0 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf05.hostedemail.com (Postfix) with ESMTP id 70A03100024 for ; Mon, 29 Jul 2024 22:25:51 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=O45VgYiu; spf=pass (imf05.hostedemail.com: domain of arnd@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=arnd@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722291909; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=jYdZqPtDr5pD+yGUFm89wpj3+F69SPbWqD0bZ/VXZRU=; b=q1oM0n8s+32m11SbzuyT/WRyC985QoW/HrcCxAdiO5PDKNWTR64gCzT1VOH+avjkkzLoZT U0OxFPPxWFtiEqtJ7WAOBZ04Itr0rmsEbSukTOdxvQ4gWg3kx3CYna32SHFPrtfJqCGbl/ A/gnQpSdJQEs/HrF2WUWQhTNKCuxh50= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=O45VgYiu; spf=pass (imf05.hostedemail.com: domain of arnd@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=arnd@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722291909; a=rsa-sha256; cv=none; b=JDSOoPXbYzP3yVn3E8/U2+X2SqTNaEh4AS62KEitpE+vhchr7t6onE0KMBFgroYYEzJwWf ihl+7LV6AonBv6ZgVelbUtaVUsbhO0+e4jsFj/z3C9rRSau5q3pnt3ywy4dt3fc7rMcbc/ R49t9HH9uvELeNAlIPJr8jdPwShOlEI= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 52E2A61C4B; Mon, 29 Jul 2024 22:25:50 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 50E95C32786; Mon, 29 Jul 2024 22:25:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722291950; bh=UYuIFJJmj4BgxktN1lSVbRHAo0jV6eeqWnYdfES+CJ4=; h=In-Reply-To:References:Date:From:To:Cc:Subject:From; b=O45VgYiuI9hf4EsHt24+2Jm0q1c4F8jLAxAf0z8sgKJPQTA/7d1RIn+YucTCEkamu F4H/iC/hNc96Cs+W/zjKgPgvA/K0LbFHLe25ZSuKUCLd4SrZwvuYvcm10bYMkRzKOK cR/Wd7RQbzzZ19bt/y6AvFKAxrwdyb1MWiPS2u+GabPSB+BEShYhElAOg2MBXVAbKS LpjaA0iUF+nBlWBKU5dAFIdyjv5wxdUwrIM4TH9OgpMNHvCI/f16HIEC/Vk8I3w4B+ P7iWmLu4TqZL0gVAtUphpKk4oAuIsR8WOQk+8QPzQ2x8g8GcHrr/3RyHWpnY5z+gC5 y3gBJnK2ed6tw== Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailfauth.nyi.internal (Postfix) with ESMTP id E1AA11200066; Mon, 29 Jul 2024 18:25:46 -0400 (EDT) Received: from imap51 ([10.202.2.101]) by compute4.internal (MEProxy); Mon, 29 Jul 2024 18:25:46 -0400 X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrjeefgdduudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefofgggkfgjfhffhffvvefutgesthdtredtreertdenucfhrhhomhepfdetrhhn ugcuuegvrhhgmhgrnhhnfdcuoegrrhhnugeskhgvrhhnvghlrdhorhhgqeenucggtffrrg htthgvrhhnpedvveeigfetudegveeiledvgfevuedvgfetgeefieeijeejffeggeehudeg tdevheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpe grrhhnugdomhgvshhmthhprghuthhhphgvrhhsohhnrghlihhthidquddvkeehudejtddv gedqvdekjedttddvieegqdgrrhhnugeppehkvghrnhgvlhdrohhrghesrghrnhgusgdrug gvpdhnsggprhgtphhtthhopedt X-ME-Proxy: Feedback-ID: i36794607:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id B043EB6008D; Mon, 29 Jul 2024 18:25:46 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.11.0-alpha0-582-g5a02f8850-fm-20240719.002-g5a02f885 MIME-Version: 1.0 Message-Id: In-Reply-To: References: <402c3c617c29465c898b1af55e3c6095@AcuMS.aculab.com> <5cd3e11780df40b0b771da5548966ebd@AcuMS.aculab.com> Date: Tue, 30 Jul 2024 00:25:24 +0200 From: "Arnd Bergmann" To: "Linus Torvalds" , "David Laight" Cc: "linux-kernel@vger.kernel.org" , "Jens Axboe" , "Matthew Wilcox" , "Christoph Hellwig" , "Andrew Morton" , "Andy Shevchenko" , "Dan Carpenter" , "Jason A . Donenfeld" , "pedro.falcato@gmail.com" , "Mateusz Guzik" , "linux-mm@kvack.org" , "Lorenzo Stoakes" Subject: Re: [PATCH v2 1/8] minmax: Put all the clamp() definitions together Content-Type: text/plain X-Stat-Signature: 6xcutoeesdoo163ocnenewrzgu8ygbf6 X-Rspam-User: X-Rspamd-Queue-Id: 70A03100024 X-Rspamd-Server: rspam02 X-HE-Tag: 1722291951-539006 X-HE-Meta: U2FsdGVkX183gNUBieYhcw6nFG5CzObziNk3vGZ7VRXNB4A2VombkWdTh12TQkTTe6ZQRm26LAtSi7oD1aawNEgUFvo+a9vB4AtbQ5BUpe4oJeepH2hK0iE8x9KaqjafQjqrtr1tFZGdG+krtksQCFq470ytSPiPJ3QvpMUL3Cim4SE+JILS5tjDumZzHlMPwDZydRAKMBZ3tWg29vs1Uw+TCZDFNkh1puerjuaVlsY6WdcdooPjRf/PBbW2S9fgdGrvqtYAT0sZZtvz/3/wO2hGOLa2X8lOEwFX4wT9d+yHqrxP3ldFWnQgVvI10lGp/R3JmRzZrVtklUsnPsnhvtZhV1cJowCDmnQe5ZGaQxNKeym1d6ejud4/JpInUTS4lU78Dq7uxrz4Wh12abp3+i3sRp3JnO6ERKDWFkD5GqY2gc9n836/KRlUc6hE7pL3LWxXeJBYjKLFZYmyg3Ew+5M56rHFzRZjc/6f2dv9TOZD33aT3SQ6hPinfnJZJyBpUqfCmAVVFOvSNLEbjRVFuKaKjVcFhV3rqtMLvrqhfhqf92+XmgXrTg520P/BY7ib8ENNyrtSjG6xrW1rC0yPzEf95V8V8CiFW7zSYBqyP+8TAs/FIksyZ5g4acWzKrZAbukwR/VEOAy13UidoydyoKJ2UHnrjDg06jGglN5mvkQKWA7n5/3tgwRgeY3fyQcjJZjI1ZUrF2UjUNqq00bo/N+8Ks2CLvllXgxk4n66frF0I2M9ACivQTj+90YOHevI/pwo/ssYjqD74wehYNbQ0pw4xWLitoY+ix26oBRo5ejlL/pxQdB7obRCgZj2nMVeAcZ+Y1ixWZ/G52vUfVnd4vAv1CqhqdH0cLH+ZhKWhw92V//SBUbyYaAiltvQJp9sCawL10QxynXCrUlS4ty0e5rRNzpE8SFaoLElmbY9bzqXqYiIVZ1oEaCws/SKxGg1PwzDk0WUau/B/Lo7zbl +HaISGuD ZPmXB4g9OOQ48E+4KAMqcYu7lZHjCvaBiQQFBGiWpqkKdKe+NI2DXJvu6PvHXDzQFiW7FY2j0NuatDpa8/hJ+MFOK+ujWSoODKCOm1F31Kzyts7M7OFE8WqsxCsEPvjA2bZ206AlbtwNCyW4RkbYftNV+G4kALce+6zmQlgLySAT71Yr70b3N01Fs5RejdWggiIrVkMZKQTlfkH33CoX/6k5O8W8V0zkZ39h8V4nKcuL2hAdM5dtYHl8I3D8yFtqdM9b6MTzjACektfuXep4rN4zjGpov9m3ik8D/HcagDQ147iC30SSkqoFBMSaGYoH+pEsCgmyh6z3WFB2zQxkE9Su1WewkM++So4QC0ksdJ0PJ/XNM4r3BM4aRDQ== 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: On Sun, Jul 28, 2024, at 22:13, Linus Torvalds wrote: > On Sun, 28 Jul 2024 at 13:10, David Laight wrote: >> >> I think they just need to be MIN_CONST() (without the casts). > > I'll just convert the existing cases of min_t/max_t to MIN_T/MAX_T, > which I already added for other reasons anyway. > > That makes min_t/max_t not have to care about the nasty special cases > (really just array sizes in these cases, and they all wanted MAX_T). I had prototyped something similar end of last week but didn't manage to get my version out to you before the weekend. Comparing mine with what you ended up committing: - You found exactly the same array index uses I found in randconfig testing, so I'm not aware of anything missing there. - My macros use __builtin_choose_expr() instead of ?: to ensure that the arguments are constant, this produces a relatively clear compiler warning when they are not. Without that, I would expect random drivers to start using MIN()/MAX() in places where it's not safe. - I went with the belts-and-suspenders version of MIN()/MAX() that works when comparing a negative constant against an unsigned one. This requires expanding each argument four or five times instead of two, so you might still want the simpler version (like MIN_T/MAX_T): --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -295,12 +271,18 @@ static inline bool in_range32(u32 val, u32 start, u32 len) do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0) /* - * Use these carefully: no type checking, and uses the arguments - * multiple times. Use for obvious constants only. + * These only work on constant values but return a constant value that + * can be used as an array size */ -#define MIN(a,b) __cmp(min,a,b) -#define MAX(a,b) __cmp(max,a,b) -#define MIN_T(type,a,b) __cmp(min,(type)(a),(type)(b)) -#define MAX_T(type,a,b) __cmp(max,(type)(a),(type)(b)) +#define MIN(x, y) \ + __builtin_choose_expr(((x) < 0 && (y) > 0), (x), \ + __builtin_choose_expr((((y) < 0 && (x) > 0) || (y) < (x)), (y), (x))) + +#define MAX(x, y) \ + __builtin_choose_expr(((x) > 0 && (y) < 0), (x), \ + __builtin_choose_expr((((y) > 0 && (x) < 0) || (y) > (x)), (y), (x))) + +#define MIN_T(type,a,b) (type)__builtin_choose_expr((type)(a) < (type)(b), (a), (b)) +#define MAX_T(type,a,b) (type)__builtin_choose_expr((type)(a) > (type)(b), (a), (b)) #endif /* _LINUX_MINMAX_H */ - The change above requires changing a number of files that were previously using their own MIN()/MAX() macros over to using min()/max(), as they are passing non-constant values in: drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 12 ++++-------- .../drm/amd/display/dc/dio/dcn20/dcn20_link_encoder.c | 9 +-------- .../drm/amd/display/dc/dio/dcn31/dcn31_dio_link_encoder.c | 8 ++------ .../drm/amd/display/dc/dio/dcn32/dcn32_dio_link_encoder.c | 6 +----- .../drm/amd/display/dc/dio/dcn321/dcn321_dio_link_encoder.c | 4 ---- .../drm/amd/display/dc/dio/dcn401/dcn401_dio_link_encoder.c | 8 -------- .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 13 +++---------- .../drm/amd/display/dc/dsc/dc_dsc.c | 9 +-------- .../drm/amd/display/dc/link/protocols/link_dp_capability.c | 13 +++---------- drivers/gpu/drm/amd/display/modules/hdcp/hdcp_ddc.c | 11 ++++------- drivers/gpu/drm/radeon/evergreen_cs.c | 9 ++------- drivers/platform/x86/sony-laptop.c | 4 ++-- kernel/trace/preemptirq_delay_test.c | 2 +- lib/decompress_unlzma.c | 7 ++----- 14 files changed, 26 insertions(+), 89 deletions(-) Changing these is probably a good idea regardless. - I also tried simplifying __types_ok() further, which as you already mentioned doesn't easily work with pointer arguments. Again we could work around this with a separate min_ptr()/max_ptr() helper. I only found 11 files that actually compare pointers (on x86/arm/arm64 randconfig): arch/arm64/kvm/hyp/nvhe/page_alloc.c | 2 +- crypto/skcipher.c | 2 +- drivers/gpu/drm/drm_modes.c | 2 +- drivers/infiniband/hw/hfi1/pio_copy.c | 4 ++-- drivers/irqchip/irq-bcm7120-l2.c | 2 +- drivers/spi/spi-cs42l43.c | 8 ++++---- fs/ntfs3/lznt.c | 2 +- lib/lzo/lzo1x_compress.c | 2 +- mm/kmemleak.c | 4 ++-- mm/percpu.c | 2 +- net/ceph/osdmap.c | 6 +++--- 11 files changed, 25 insertions(+), 18 deletions(-) The simpler __types_ok() needs more testing across all compiler versions, so that wouldn't be for 6.11 anyway. I can send the min_ptr()/max_ptr() stuff anyway if you think that's a good idea. Arnd