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 A92BAC02182 for ; Mon, 20 Jan 2025 11:21:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2495C6B0089; Mon, 20 Jan 2025 06:21:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F8F06B008A; Mon, 20 Jan 2025 06:21:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 09A666B008C; Mon, 20 Jan 2025 06:21:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id DC9906B0089 for ; Mon, 20 Jan 2025 06:21:31 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 87EBCAE16B for ; Mon, 20 Jan 2025 11:21:31 +0000 (UTC) X-FDA: 83027589582.20.FDBCE6C Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) by imf11.hostedemail.com (Postfix) with ESMTP id 5761940010 for ; Mon, 20 Jan 2025 11:21:28 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HXPYvxJC; spf=pass (imf11.hostedemail.com: domain of jani.nikula@intel.com designates 192.198.163.15 as permitted sender) smtp.mailfrom=jani.nikula@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1737372089; 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=CBclP9j8M5XS+UpdQ2YDlpUxElta+kfFfrKNZHdDWhs=; b=zid/Saiy0LPQ58YI211xxvrRiKW1d0cFh0OcQfb53SAJPkVYRZjGKXriK0/V2Llzfv2uJn VwEtWf/ahL4Kx+VlcXpFhdR+TrPWUoRIfUSn1/LALuHD00Nui8K704AruCIQf71GH9TfyP uwzPgOwoYpxMOUr1SrEJY8RMQS5kSAo= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=HXPYvxJC; spf=pass (imf11.hostedemail.com: domain of jani.nikula@intel.com designates 192.198.163.15 as permitted sender) smtp.mailfrom=jani.nikula@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1737372089; a=rsa-sha256; cv=none; b=ZOLboKBDdvsQ3ou4fqm1iT0gOiNbLhFJxRyBGOiXR7oH+kq188Ky5tvWlnPRnLExlLDKfe iWKs9DAOjyUzp2fW+jROmTlHz3UEEggED6cMOMCfdJuXDAhpU8dn1Y44SzVaZFPyWIM3x1 O7EF6f/w8Rp9sAPvEPjuCmeu1sCS7Lc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1737372088; x=1768908088; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=Bqm7nHO1/aCxnCIrdM+FoX3RzC8TchksMVJHfy7cAHU=; b=HXPYvxJCJsQLwgKpjr/P7OdOyMZ8oAMT5vqVwHIDKDRblftdlASJCUip JsriXvqx0nsfZWxXOLym9ui4Xoo3s8o0jCBPu9VjxfSMMXAJsVP4gnKQl mt7Q9luZvURSz87BBkebosAMfp9ZqVkZZKAZiUncpy2kg038jFMHJ7OaU HWNAQOu+sBCvc9HHz+Hdxn1DPotLKZObFM9p4Bg1fe11b3Sw77k8VfGeE IfSQZo7Di+9T6lBJOF0e6fqZpWmlChGjc2fkSbEVJrMoCsuasGpOWuSam v7Q6shmmj/GXuXBDxGeZyt1qca5xwoonWsJNWZn4NXyukuNmLKXF49Vou A==; X-CSE-ConnectionGUID: 4VID6ij3QxKpWH94i18RWQ== X-CSE-MsgGUID: iyO0iZ95R2SAQPMBT0a5Wg== X-IronPort-AV: E=McAfee;i="6700,10204,11320"; a="37917871" X-IronPort-AV: E=Sophos;i="6.13,218,1732608000"; d="scan'208";a="37917871" Received: from fmviesa010.fm.intel.com ([10.60.135.150]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2025 03:21:27 -0800 X-CSE-ConnectionGUID: MvDLfc5CTomhxALmG/Ajug== X-CSE-MsgGUID: SntxViPMT/+ezMNA1rOSfQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,218,1732608000"; d="scan'208";a="107017049" Received: from mjarzebo-mobl1.ger.corp.intel.com (HELO localhost) ([10.245.246.106]) by fmviesa010-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2025 03:21:20 -0800 From: Jani Nikula To: David Laight Cc: Guenter Roeck , Linus Torvalds , David Laight , Arnd Bergmann , "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 , intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, David Airlie , Simona Vetter , Rodrigo Vivi Subject: Re: Buiild error in i915/xe In-Reply-To: <20250120111551.435176c4@pumpkin> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <34d53778977747f19cce2abb287bb3e6@AcuMS.aculab.com> <20250118170959.3aa56f4d@pumpkin> <29ef57a1-e4dd-4d5d-8726-f1f79c698b66@roeck-us.net> <20250118221123.5bb65e64@pumpkin> <20250119090935.7c690f85@pumpkin> <87ed0xrcb8.fsf@intel.com> <20250120111551.435176c4@pumpkin> Date: Mon, 20 Jan 2025 13:21:17 +0200 Message-ID: <878qr5ras2.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 5761940010 X-Stat-Signature: 9zp59kf9be4nk4zc9irfto81jd19tjq5 X-HE-Tag: 1737372088-413146 X-HE-Meta: U2FsdGVkX1+LcRigiFJGlvscJu0mZCsgXZVqPKyA5F5505mdzzyNhBsi6ULBz8Uxyg1hNoBibk6xF/yq6iiRQGdXn5pWUuzL6rKtAU136yNgIW0a4i1PVnUWCIlqnbSevLlouM11gnvaIwL9r7ZPjWI3yoO2VG0Za9tqWeaVImsP8Ckdx85CIzSJ39n8Nw/N8szvTKH05DzbQxJMwHiRwP+2Y6wfWxfpD35r0H5rdJBfbaLMYGQ72iAInSpK28OT6TeDLDOgt7TM2sldwoThnTD9Oxiqbpu5yJBKmm//85xv1Z6AdBf0734EwcTcggCbPR5LyUMA9mqYWM2nlafx1E6Zt78oxBAFRl9VYRtyTJx5LcoSmXMk/iMo0izQFm4wAJmm6rvaEAQzfHuNhlM2PfybbH5LT7omhTOVV4gup2vkw4n9afHTzPMmLIm+HSwzrMyp94xqeSsU69/1EjcFCOOyA9NpTciC08cRlwV0EEEr06JQVH31rlTEOOZJHyhlJv5t/m7oE4+vY5MTGhZB4y1NToA+0dSjnr+DcRPOtGVXLUUa8CmXt5p0j/I1k5I+/EMul4JV8h1c2090+ld4a8g+lDJLwHYQ18TT4TrS8jk8VMhtL7dYkgFjSt6jDKCt4aauGCIAHDE9c9AJb84421a8A1uyLHJyR3k5o51aYEkgaHPYERxCMldAS0qRTnemLKB5riPmYMCmWUubZu9QIIKjpiR3QIgQctpe3+l4Lefc1YCeEXlHKik5UP/hwRo8rSRagRnbye2pAeosQpia3C6uDiqX/tna8bU3b3Pxxp6gAxX6RLsKBOdOteX611gbcGwMcjnK94OOMKpd39V32SOaURzucihob9Uyzj7beKFXF3MjNVDeyJOQBgI4VxEeauxgze9XcpP7KtvKtiaEVG3NLbwM1BXoh/+Na1i6kzPo9dAcFTfnogr68COhHTsAsJ2zDZeM44a2MczPiG6 gLOLa6ya KPOYVyOT3qofp70TwOcb+gfWLRgOWv6Y/Y2e3uYU2dQEtUr6sjJ6eHrkoTC3USeW1/AtWpPBRGiTBCHlCJTFSH0/doAWi4VegU0fN+Kw/QC87Nusqqmge9emhlh3ZfmiZX7wMyAaMcz1KjI5JfnykOs2Bb+SBW/CLzVlIbY7GfbW6al/P+3ZR8KqxaF6cgyD75fpd+8lO+LQDJlPeQBVT95ea58S48S7y2Rh7x8SMD4GE6M/9c1pM1eW4YfA8hMNJVSLtvWRXUOKITx+RJFI9mvMM49XNdk78zejxI5mOgHbnG48Mqb8P9j0gcIzT8wiMxyI2icf9zY+QRUxV9KwsXnHRS1k2IhekGJD0Ow+xVuso7XYf+P6i2PJ7a8bQiYyXJ041lhQsZLn0WC3vIfff0MOi5sdnbuNeioAzzEJVFczG/gqENx3CjXinUfY0Q/Eu8IoVGRTsOPKYNaxAZnHYpUecR5qvMTCbQM/2ueocX3AwK+5cXCCYuOfEZRPW0aCBgsOXHMTXmHMT4zzB7h7jz9SchuW96gD4/b0y3Buzr/m7DalOU1DVbWX5/GoKehn1Og0uJruxfOfIK0Uu1XahjPwgUgsYSMk1mH+3cbgWtmO7nY4BTn/8ILVmRRW6XFCiB67/ 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 Mon, 20 Jan 2025, David Laight wrote: > On Mon, 20 Jan 2025 12:48:11 +0200 > Jani Nikula wrote: > >> On Sun, 19 Jan 2025, David Laight wrote: >> > On Sat, 18 Jan 2025 14:58:48 -0800 >> > Guenter Roeck wrote: >> > >> >> On 1/18/25 14:11, David Laight wrote: >> >> > On Sat, 18 Jan 2025 13:21:39 -0800 >> >> > Linus Torvalds wrote: >> >> > >> >> >> On Sat, 18 Jan 2025 at 09:49, Guenter Roeck wrote: >> >> >>> >> >> >>> No idea why the compiler would know that the values are invalid. >> >> >> >> >> >> It's not that the compiler knows tat they are invalid, but I bet what >> >> >> happens is in scale() (and possibly other places that do similar >> >> >> checks), which does this: >> >> >> >> >> >> WARN_ON(source_min > source_max); >> >> >> ... >> >> >> source_val = clamp(source_val, source_min, source_max); >> >> >> >> >> >> and the compiler notices that the ordering comparison in the first >> >> >> WARN_ON() is the same as the one in clamp(), so it basically converts >> >> >> the logic to >> >> >> >> >> >> if (source_min > source_max) { >> >> >> WARN(..); >> >> >> /* Do the clamp() knowing that source_min > source_max */ >> >> >> source_val = clamp(source_val, source_min, source_max); >> >> >> } else { >> >> >> /* Do the clamp knowing that source_min <= source_max */ >> >> >> source_val = clamp(source_val, source_min, source_max); >> >> >> } >> >> >> >> >> >> (obviously I dropped the other WARN_ON in the conversion, it wasn't >> >> >> relevant for this case). >> >> >> >> >> >> And now that first clamp() case is done with source_min > source_max, >> >> >> and it triggers that build error because that's invalid. >> >> >> >> >> >> So the condition is not statically true in the *source* code, but in >> >> >> the "I have moved code around to combine tests" case it now *is* >> >> >> statically true as far as the compiler is concerned. >> >> > >> >> > Well spotted :-) >> >> > >> >> > One option would be to move the WARN_ON() below the clamp() and >> >> > add an OPTIMISER_HIDE_VAR(source_max) between them. >> >> > >> >> > Or do something more sensible than the WARN(). >> >> > Perhaps return target_min on any such errors? >> >> > >> >> >> >> This helps: >> >> >> >> - WARN_ON(source_min > source_max); >> >> - WARN_ON(target_min > target_max); >> >> - >> >> /* defensive */ >> >> source_val = clamp(source_val, source_min, source_max); >> >> >> >> + WARN_ON(source_min > source_max); >> >> + WARN_ON(target_min > target_max); >> > >> > That is a 'quick fix' ... >> > >> > Much better would be to replace the WARN() with (say): >> > if (target_min >= target_max) >> > return target_min; >> > if (source_min >= source_max) >> > return target_min + (target_max - target_min)/2; >> > So that the return values are actually in range (in as much as one is defined). >> > Note that the >= cpmparisons also remove a divide by zero. >> >> I want the loud and early warnings for clear bugs instead of >> "gracefully" silencing the errors only to be found through debugging >> user reports. > > A user isn't going to notice a WARN() - not until you tell them to look for it. > In any case even if you output a message you really want to return a 'sane' > value, who knows what effect a very out of range value is going to have. The point is, we'll catch the WARN in CI before it goes out to users. BR, Jani. > > David > > -- Jani Nikula, Intel