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 24D09E776CB for ; Mon, 2 Oct 2023 22:51:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4C1BF6B011C; Mon, 2 Oct 2023 18:51:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 471BC6B011D; Mon, 2 Oct 2023 18:51:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 312056B011E; Mon, 2 Oct 2023 18:51:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 20F9E6B011C for ; Mon, 2 Oct 2023 18:51:10 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id DF97E1A00DF for ; Mon, 2 Oct 2023 22:51:09 +0000 (UTC) X-FDA: 81302018658.13.7AAAFAF Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by imf05.hostedemail.com (Postfix) with ESMTP id F3C5C10000C for ; Mon, 2 Oct 2023 22:51:07 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=f+rUbeXF; spf=pass (imf05.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.48 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696287068; a=rsa-sha256; cv=none; b=AUuVK9b9wc3BQ0Sl6YuVVb0Wf6PCzcIJNKBMWe4NxJo0rGiuPeWMlGEWS98dfQbrQaGBm4 OQ5iZgD0m23LAlnawbJujjBOYlyUAhxCn0842wz0Afn/+9uGEcMAPjwafHPizFt0hoBRMd I6qeFfBPNG/vV3p1AJRtNKvfz+zSL2g= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=f+rUbeXF; spf=pass (imf05.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.48 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696287068; 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=PiBVYtIuOlLuFNiBVRUMfSfexf+qnE+0Tc8CZdcfjOg=; b=ttyE/hCUWzkPv32cXixOC+eV8vikiNDo2dn11w7WU3vjntBON4SHxoz6OUQ/S0CoKEoWp/ YwLx3YusiqErAo5lpjTvaFesKl6sv+vrqFTtFNJ1pcph31GMPMUWmixw1Z/2KdaXod5EGh DTlrxnvi/5d7vo7ku9hB0Ums8FjZd6E= Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-40566f8a093so2881785e9.3 for ; Mon, 02 Oct 2023 15:51:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696287066; x=1696891866; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=PiBVYtIuOlLuFNiBVRUMfSfexf+qnE+0Tc8CZdcfjOg=; b=f+rUbeXFy4swHeXDEe2bOg9f+SsmLhQfA+mlq2mQkA5q2hkXC4rjSOXw/sEpqheHZo 4LT0L20DoCvS6nEe2QQJDJGBU+rOPoJ/ke6GyPaSggt8Vkk7Ss1/LtV4k9GqCG1JUijN YsAGtMmDeLVZ6GE9jpxth3cuyxeNYkYud+5NDOoklf6v4UFfiHEVOI/MHkWTXZUSPdGa pU7ffSe/8gTmHrMQRQYNXG+CwwU0g8H9XL5swrh0pfebppV6hCGEAeymqzCQRWs9sfXa oXFQRJtFnQGWKcjfp1HU9XAMqX8iqGI4b1/G6DKoI/CfO87sX6Y55egKgD2C9vBO1YCF T2Zw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696287066; x=1696891866; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=PiBVYtIuOlLuFNiBVRUMfSfexf+qnE+0Tc8CZdcfjOg=; b=gALsoXKU2KHbVyJsDKOyZToLKEWEc6bAt5htaxY7VWDKRF25Wj4N95/vj0v3/A6pEn hev98c69PbfupJE40PAJ3CbS7lhaPnyLU7Yk3L/sF8hms/+uP4CqbDJpMb7f0yRw11gL cxu6i49DSrOVabfNYBlVmCpKsz6FaSMmArGnmxLlzg5fwEbFt+DkohYyfTi8pyOHFF2F V5at6daDNfWXStztApcX0Ey37pfjnXoOmZy8czCVOfO3mwrpe+r414QWfXYNMNr4QozZ ajPCPpRLJFCjKselEmzLfSHXwzlXhr3PbLhEQ1TuQu34/MG9DIZ4/NkhzwY9t70AgLzW ZoFA== X-Gm-Message-State: AOJu0YzsWPmee44riRRxU/G2mPdKYZ4/CmDHhqKYyRFURswJoavxfL4o Y1keZfRKNJiuAPA5TgfFLpU= X-Google-Smtp-Source: AGHT+IEqBIX71QHlHmjXxlsBxqravil/edeaibGDoUYymDPdHZAtqf7Z5cLRPMS2JvwRgC2LpRcY9A== X-Received: by 2002:a05:600c:2113:b0:405:1ba2:4fd1 with SMTP id u19-20020a05600c211300b004051ba24fd1mr12423351wml.24.1696287066108; Mon, 02 Oct 2023 15:51:06 -0700 (PDT) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id f14-20020a7bc8ce000000b00402d34ea099sm8045752wml.29.2023.10.02.15.51.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Oct 2023 15:51:04 -0700 (PDT) Date: Mon, 2 Oct 2023 23:51:04 +0100 From: Lorenzo Stoakes To: David Hildenbrand Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, 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 , Oleg Nesterov , Richard Cochran , Jason Gunthorpe , John Hubbard , Arnd Bergmann Subject: Re: [PATCH 3/4] mm/gup: make failure to pin an error if FOLL_NOWAIT not specified Message-ID: <6a421da0-8479-4873-8e46-6f92aed342c6@lucifer.local> References: <6161e8a8-64a4-c4ea-626d-daac45ccd836@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6161e8a8-64a4-c4ea-626d-daac45ccd836@redhat.com> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: F3C5C10000C X-Stat-Signature: nqt1earapwrw1ucxc4p3tcrysfzwfyxz X-Rspam-User: X-HE-Tag: 1696287067-47304 X-HE-Meta: U2FsdGVkX1/j8Ney8J2RQuIyLD3ot4hwO1Cy4R3sQ/h9+SwuZebXXuKJgx0FTZdTScjD1MnjpOQ67IC02YKI4uSSSeg6SdbZbRvEOwmeHd1NRps+rnaombo6qik1MDBjsv7LUZ+09o/ZRVOnMU75ReWKXRHnsCskebeVNAIpwvuQD+G0ma33ExfURrINvAXHA6Wng8G6orBpDYlJBpoKE4EXFNbK9q462Q9OxVJMjpa2bn5Ax7IOWJD+bn3+rHpVqf2BQdiFAnED6F57c5M+nEhi2VfXdD7RYgBzDCUPtZYGA/C9nnqujfyr5MASRtikhfBW+NVRdxrtWVrwnL0NHzX/PF42mdx8VTJR3xZ2h4hv6oBhMw0a6iCK3lVX5HBtRml7HCQUczCUr+7e0vSuKhKTvpT+uQgy9Yt30+4uWSyVHaHmv8pfLUp4flix67v9XrWS2ueYb9y9qB6BBFBlavl5b+KYGDiFePS3IrA+0uLJA8zzkf2PWiuBmvlh6cgF+BIr6N4ZCWuPf3XG0EBQfx2hDYx9zSlXVv+RAoCairxfUYJkmMJE4DxbuAMxBWrdhzdkA7mLrdWzKBbMlE6ILMtc/EakoCjwibLBRMlXeWtRxV0c/2dhm1G612G1cxk/rQ+LAmoRiGzoA++Umw8R7mcV2HWUBmw3VE0NIb+unmOYvoPkPYfIXYQ0qPlliQIOlTyiicaMneaEUcR5MnaYTw8sUKfZBUHg4kuxFKFSreMdNvHspAMYNfD7TvB/sAuCTkbVFsWRa16XlBL6056BrxpV5A2u2l5HT/Tts1kXDWSau5Sqif6UNoG0aS+U85XyQYVWibL4XHK+f7F1DUJ4KaEfsRbDUMQsBrdN0xhNDDuTbud7IUn3GIK1IDVe2ZzJ0oPBz4kD+704UsLvrdRbbGt1vYq+2jFbF/iLTGh+qD4VcUK1ZiEGCBSfu5MMDJaFqhvLX/GeATIJKs2rGHP oFNB2s22 RUIU+PwsJan6JO0lOw9DR+TIKpaB8/Wjkvu+Ryv3w2F/vLKD+cZ5LpP0NrTa8sw0bgdX9ICQWUC6s6ZrwgNltXxNjqxY783oC4EuWcWFZIP6Mr1Obi5nFLQqv0yGk+GkqwSPK3757VaTN95tZVVSIXJRl+bu+Cb+J+Y/rZzXY2ugC6GSEk8o6S0M+zmgsgblrqfVm+CKPsXOze3h9vaR70omWf0nZNbHxVM2hljSOHyl5Pdxr3ySTiRW3/IlaZbgK7XoLDgfWq5YTPn3J3qik6jo6Gy5lCGpNu647pbgDSdA36WlPWy1YNvuWSdqpKaym0wy9/HiL+GUsXX8ftLZ05hNCa5fUqQCIZrkq3fU09AVSsVNGDTG0UZyBdiwzeX4halMkmkdfflfsCahrIr2I4gCh8zXqUaVr1oNfyLkkT8W0Za7l65RtuQK8bm5FkSdkllI8vEEPVHy5htc/d2P/qmV3nSuKPc7cVsEc 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: On Mon, Oct 02, 2023 at 01:04:51PM +0200, David Hildenbrand wrote: > On 01.10.23 18:00, Lorenzo Stoakes wrote: > > There really should be no circumstances under which a non-FOLL_NOWAIT GUP > > operation fails to return any pages, so make this an error. > > > > To catch the trivial case, simply exit early if nr_pages == 0. > > > > This brings __get_user_pages_locked() in line with the behaviour of its > > nommu variant. > > > > Signed-off-by: Lorenzo Stoakes > > --- > > mm/gup.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index b21b33d1787e..fb2218d74ca5 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -1471,6 +1471,9 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm, > > long ret, pages_done; > > bool must_unlock = false; > > + if (!nr_pages) > > + return 0; > > + > > Probably unlikely() is reasonable. I even wonder if WARN_ON_ONCE() would be > appropriate, but likely there are weird callers that end up calling this > with nr_pages==0 ... probably they should be identified and changed. Future > work. > > > /* > > * The internal caller expects GUP to manage the lock internally and the > > * lock must be released when this returns. > > @@ -1595,6 +1598,14 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm, > > mmap_read_unlock(mm); > > *locked = 0; > > } > > + > > + /* > > + * Failing to pin anything implies something has gone wrong except when > > + * FOLL_NOWAIT is specified, so explicitly make this an error. > > + */ > > + if (pages_done == 0 && !(flags & FOLL_NOWAIT)) > > + return -EFAULT; > > + > > But who would be affected by that and why do we care about adding this > check? > > This smells like a "if (WARN_ON_ONCE())", correct? Sure it does somewhat, however there are 'ordinary' (maybe) scenarios where this could possibly happen - FOLL_UNLOCKABLE and __get_user_pages() returns 0, or lock retained for non-FOLL_NOWAIT scenario and __get_user_pages() 0 also. So I think the safest option might be to leave without-WARN, however you could argue since we're making it an error now maybe we want to draw attention to it by warning. I just want to avoid a warning that _might_ be a product of a particular faulting scenario. Jason or John may have an opinion on this. Actually having written all this, given we're changing this into an error now anyway and this is just not a correct or expected scenario, yeah I think WARN_ON_ONCE() would make sense, will update on v2. > > -- > Cheers, > > David / dhildenb >