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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 53C9FC77B75 for ; Fri, 19 May 2023 15:10:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Subject:Cc:To:From:Date:References: In-Reply-To:Message-Id:Mime-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sEdz+1up/Iu1kEQKh9/6qwYRBxW3Jyji3b2oTr0024o=; b=dHts8DbYDGVK9f luh2eQz87SWBRe3o9BR57+ntoFYiDHmFgl82HstfTu0KVUlkTCQVOl5jNmi+owe9xLmo8YZN+Hbib JHrKT+t+aWIYpWgUIGf+AwEN0/ML4gY2w6h91zFWFVwERsNzMvc8rvXi1oQAvV4hzt4ZUMJm7riod 0IutsSIbyeZDY+kgTUpsr+m6bcCYx0QlEJsZJUHmx0PCwrLVtx5JXxwGCb1FNt9HHKelzfem6rH80 CauVJFh3e7OZB89ZFnT1Auw28+Y5kZ3txSJW/gr0kZduk9OANZi/JAJHpg5LkXj82poWS8whhkpOz wmBoOcsOFfHPOsOHPVkg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q01k4-00GZM1-35; Fri, 19 May 2023 15:10:04 +0000 Received: from out4-smtp.messagingengine.com ([66.111.4.28]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q01k1-00GZK7-1V for linux-arm-kernel@lists.infradead.org; Fri, 19 May 2023 15:10:03 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 78D6E5C01E4; Fri, 19 May 2023 11:09:57 -0400 (EDT) Received: from imap51 ([10.202.2.101]) by compute6.internal (MEProxy); Fri, 19 May 2023 11:09:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arndb.de; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm3; t=1684508997; x=1684595397; bh=Hd AFOpP4Mmpj8sP8nf+XQRyuNt5e1ngGbSBEuPLR87Y=; b=K78d4ZcA+QFwcBlStc cQQ4iYXDop4aIpgBuKqwKvLt25kPy3BtCpKOgNfoIFMw7PGm8cZ9BjQmPuHlVBt5 MUihYz2EDPttZvPEQKzYhWoFzJ4osztmjl6rOwuW4eXQuP/YMYZ2XCZNSxU2iyaU Sxil6LPHtoVjM+CoOmHmqU6k++qMY10ZHJK4jwT2iSt9Hupfr+wuAIEuhsycZwFV 8EcUoeeZ7INOot/zOejk2HazHxI7B+Lop4oIoyVU0isx1e+bhUbHj1yFQlYZPacW XuuqUeaAEoqNaydIRHBXL0Y2QO8UfG7SJ1Hva14MAtfxfdJ7vsrurQFbhz5OG7Y7 XIvA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; t=1684508997; x=1684595397; bh=HdAFOpP4Mmpj8 sP8nf+XQRyuNt5e1ngGbSBEuPLR87Y=; b=ftIfzBI9DALcM4R1kBB4Wvk+fT0eA jH509uhYkuKVC234j+xnAggzTeQXyOFTKSYxQBSlHbEKI7HrZECONql3zCZ/nEzK OO52dzJKuab34buXQH2Kq1smsfapoWBdYToodvYgdZig7e0v3/2G5isnIvmoXXwy 802paETXLUhfJcfK+kgpNIQK4ZJDJulv+UxTjapN+L+jHfYTHOVXskB+dE1/eOF9 lZdtNDgfCdNvuFOF11obm4BnzWPRjUHTCS5wvp+RBTgjBA8yy6tC2a2GCdTHoYuo OIe8U2MCwqLwM2JigAoGdxDvb0BWamTTzjJ7d09CsN8bCcgIyYjVXyvgQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeeihedgkeehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepofgfggfkjghffffhvfevufgtsehttdertderredtnecuhfhrohhmpedftehr nhguuceuvghrghhmrghnnhdfuceorghrnhgusegrrhhnuggsrdguvgeqnecuggftrfgrth htvghrnhepffehueegteeihfegtefhjefgtdeugfegjeelheejueethfefgeeghfektdek teffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomheprg hrnhgusegrrhhnuggsrdguvg X-ME-Proxy: Feedback-ID: i56a14606:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 39E83B6008D; Fri, 19 May 2023 11:09:56 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-431-g1d6a3ebb56-fm-20230511.001-g1d6a3ebb Mime-Version: 1.0 Message-Id: <7d7ddc48-5985-4678-9f87-6e9b574a24d9@app.fastmail.com> In-Reply-To: <5b071f65-7f87-4a7b-a76a-f4a1c1568ae7@lucifer.local> References: <20230519093953.10972-1-arnd@kernel.org> <5b071f65-7f87-4a7b-a76a-f4a1c1568ae7@lucifer.local> Date: Fri, 19 May 2023 17:09:35 +0200 From: "Arnd Bergmann" To: "Lorenzo Stoakes" , "Arnd Bergmann" Cc: "Andrew Morton" , "Catalin Marinas" , "Will Deacon" , "Peter Zijlstra" , "Ingo Molnar" , "Arnaldo Carvalho de Melo" , "Mark Rutland" , "Alexander Shishkin" , "Jiri Olsa" , "Namhyung Kim" , "Ian Rogers" , "Adrian Hunter" , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-perf-users@vger.kernel.org Subject: Re: [PATCH] [suggestion] mm/gup: avoid IS_ERR_OR_NULL X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230519_081002_095066_40E84122 X-CRM114-Status: GOOD ( 21.58 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, May 19, 2023, at 16:51, Lorenzo Stoakes wrote: > Given you are sharply criticising the code I authored here, is it too much > to ask for you to cc- me, the author on commentaries like this? Thanks. My mistake, I expected this to get added automatically based on the "Fixes:" tag, I probably dropped you by accident in the end. > On Fri, May 19, 2023 at 11:39:13AM +0200, Arnd Bergmann wrote: >> From: Arnd Bergmann >> >> While looking at an unused-variable warning, I noticed a new interface coming >> in that requires the use of IS_ERR_OR_NULL(), which tends to indicate bad >> interface design and is usually surprising to users. > > I am not sure I understand your reasoning, why does it 'tend to indicate > bad interface design'? You say that as if it is an obvious truth. Not > obvious to me at all. > > There are 3 possible outcomes from the function - an error, the function > failing to pin a page, or it succeeding in doing so. For some of the > callers that results in an error, for others it is not an error. > > Overloading EIO on the assumption that gup will never, ever return this > indicating an error seems to me a worse solution. The problem is that we have inconsistent error handling in functions that return an object, about half of them use NULL to indicate an error, and the other half use ERR_PTR(), and users frequently get those wrong by picking the wrong one. Functions that can return both make this worse because whichever of the two normal ways a user expects, they still get it wrong. > Not a fan at all of this patch, it doesn't achieve anything useful, is in > service of some theoretical improvement, and actually introduces a new > class of bug (differentiating EIO and failing to pin). Having another -EIO return code is a problem, so I agree that my patch wouldn't be good either. Maybe separating the error return from the page pointer by passing a 'struct page **p' argument that gets filled would help? Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel