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 BC594C77B70 for ; Mon, 17 Apr 2023 13:23:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 51E4D6B0071; Mon, 17 Apr 2023 09:23:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4CE1F8E0001; Mon, 17 Apr 2023 09:23:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 36ECE6B0074; Mon, 17 Apr 2023 09:23:58 -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 266E36B0071 for ; Mon, 17 Apr 2023 09:23:58 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id DB1FF1A048F for ; Mon, 17 Apr 2023 13:23:57 +0000 (UTC) X-FDA: 80690950914.21.4B9EC93 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by imf22.hostedemail.com (Postfix) with ESMTP id 0CF14C001F for ; Mon, 17 Apr 2023 13:23:55 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=oNOvCHGN; spf=pass (imf22.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.44 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=1681737836; 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=RKAAc3ZsjOsQZpjk6XJI6NMJzVbDtqI98XsTFMuJ8w4=; b=F0aetcNBPwgtrrh3UnPCKujphO6Tm2Scvr+Gcj8Vk+3uY5gUHA0aGQpeY8qPtH0LGJ0v4b gUfZZg4VkxW1iwrUCAZ2WM4TD2AqJZHd45ZzitAKW93Eg1u+PthaTygsi7nyuEcL/OVDcr fbrX2ROSPb4sXpkeR3ypWtrAFmcCkaI= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20221208 header.b=oNOvCHGN; spf=pass (imf22.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.44 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=1681737836; a=rsa-sha256; cv=none; b=aeBXaX+NpVjfq5UykqZE9VCRCV2602R+H4qg4eE3WR+LapmznzDDPW3zjbWYuOMHGJn0q5 HCL6seFHhHSddtEdit+/COTG2gc5XfdP6h5lYb82WFOCr+MRmTrbkAScxrrr8xnu8ceP22 zFwukHwMIVhUcgBz7+aCCtcBccxzkoA= Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-3f16b99b990so13630965e9.0 for ; Mon, 17 Apr 2023 06:23:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1681737835; x=1684329835; 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=RKAAc3ZsjOsQZpjk6XJI6NMJzVbDtqI98XsTFMuJ8w4=; b=oNOvCHGNkSBvUbh4ERh8j/DqJtUwbQoII2LGqZbVAhWFWfbbcnRnf+8UhdDzO/d1ET i21AXWRsFFh1RWrHvwHpOF6isEiP2D2zO1ON4sIutL4dtw2gJaEBVtjK+ZX1jqOwfXxe bVmx0wrpYw1yojVfAmWruLcF69lFlLHUEQKj/58zZKf6DmefiuCIzxxqH4DJID8yaXbt urENc2lY29lF4j2ZupatM9Uz2p8FmREaiqOOYw7xEoUPXmJhrPFMpD8ihomU3gxN+/no nX01agI2B5Q0NxrW9XatoyAzGuOV0xtn3Regsk8O0VTAdr+YKUHGHmxJzeZsjHXbaZ7u 1r4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681737835; x=1684329835; 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=RKAAc3ZsjOsQZpjk6XJI6NMJzVbDtqI98XsTFMuJ8w4=; b=dG3w+90eIJDseRkt9qmntq7xjnxyOF0NjRoXvcKHVn/QAXpcg/gShbqYnklquDUH7T ipEASCV2QY8hdsf9Bjg3oGrjbl6gd59E28CeM6Z/K1Lw/GnA4rrSyn8dkQqEoFGVGYE5 AOvsFWoayLRugaTMwwl/flR9chJXoARCRy4SurxnzDuV26v2ojghxsWSkMFV/kp/v3yP bWEs7RvN1rj6ijw0Bw8p+ATBJItX3j5cC6X5N+PnvUNVPMdwGXgNCagrTwMkuuU1KPfw nP11Oq17zHRciQuvwTtxqKvF7Kb4SguXvFkA8lcBT/0ZrsDnlk3IIRJ+sGcvPDByvG4P Xodg== X-Gm-Message-State: AAQBX9cfiKwnkGUT2EjFuHFjBCwVwzYo58d9isOnmAfEgjY/IgXgm37G NfMa0TW1uhyBEpXpeDlstOU= X-Google-Smtp-Source: AKy350ZgGhmNdbORfcWWT3+kEaLh1DjBGO2Qr9T2OkuqSkpU4KmNaszZ8mszkHwAUCbDgkdgFsooIQ== X-Received: by 2002:adf:dd4b:0:b0:2ef:eb54:4dc0 with SMTP id u11-20020adfdd4b000000b002efeb544dc0mr6790292wrm.51.1681737834522; Mon, 17 Apr 2023 06:23:54 -0700 (PDT) Received: from localhost (host86-156-84-164.range86-156.btcentralplus.com. [86.156.84.164]) by smtp.gmail.com with ESMTPSA id k14-20020a5d66ce000000b002f103ca90cdsm10571529wrw.101.2023.04.17.06.23.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Apr 2023 06:23:53 -0700 (PDT) Date: Mon, 17 Apr 2023 14:23:52 +0100 From: Lorenzo Stoakes To: Jason Gunthorpe Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Matthew Wilcox , David Hildenbrand , linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, linux-s390@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org, linux-security-module@vger.kernel.org, Catalin Marinas , Will Deacon , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Sven Schnelle , Eric Biederman , Kees Cook , Alexander Viro , Christian Brauner , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Kentaro Takeda , Tetsuo Handa , Paul Moore , James Morris , "Serge E . Hallyn" , Paolo Bonzini Subject: Re: [PATCH 3/7] mm/gup: remove vmas parameter from get_user_pages_remote() Message-ID: <241f0c22-f3d6-436e-a0d8-be04e281ed2f@lucifer.local> References: <5a4cf1ebf1c6cdfabbf2f5209facb0180dd20006.1681508038.git.lstoakes@gmail.com> <9be77e7e-4531-4e1c-9e0d-4edbb5ad3bd5@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Stat-Signature: ambfhs6ibfktwhby5kpeirpbudqsnddc X-Rspam-User: X-Rspamd-Queue-Id: 0CF14C001F X-Rspamd-Server: rspam06 X-HE-Tag: 1681737835-418190 X-HE-Meta: U2FsdGVkX18f1PejmnaZkKlL/jIy04BB82rgYXFuwtV7ICaO9QGGyxUSHJo3ts7qFLqHZX9COS5kGf1c0uFoCQlYvfp8rSRwrcMS1ncwEzwFIBkNpjFyNFs5jkcKefJVdK7xOeHM0WpJg4aXfV4qhdRHO5hpM/fs5wMUOXybKS7WgbMMsvMUAbnvVwaeRP72MoI9yZdMrVD66dLPwI2+lEM340xNmGBjbc5/f1vmJTEP6fxnVzfY3uGeyZoKaZciTi9HILzr7KRbJX7sT3+FClWQXWCCD8BmHz8OjAlo2pwC5/O2+MAv4y0Gdbn7QyVHnH/+fPoJ0JoRcohaF0zc1XGSAmD5NFL+j+NgJ0tY7+JLu3UePzNdx58oYerCuT7Yg1WOXnGPkePfW6al8qOdNWY1JJSDylJJb90vdTs5NMpOdNSKlmiTyq0EfP7dyssRbJ5BfgRN6yV/orVFAQ02VubZmnW+1TqVtL97S+iodj6mltZFS7G3nnss6kotbH7KfqGu4Y0BnPh54x2blqRAcvHxGygmhUNmO6BrjxS2jt+ukR8Vn9iBXqeWbmOCrWqpaKWoTpFuf4jwyK3MNrNpas1NkQxX0wblxIXh23u4c1zChjz6o+xKIUc5wPWiF3JeSSNxNjJqjwW7TWzaZI0D53dTDXx16JeLULHf/utddNFEd9c5wvfnd6y1hLRRlNhm9ROtEznjYfYjVEvhy7GDkxFEU2XL88WSREzrES7nRT2RdW9vJRob0TyWO2JdPA02KcOq6KU+r4HCM4C+QLF6X2f/JIHe3g44Whlyq5op9sdFqo9wsKFVhUbQ19oEPhP0kP2G0LZQXrDBNUECooX8EZjbLbUnmr0XNp3CVtAcRkvENjo039W7DURniNgsCh24c2EwoniBL8Yc+t4+/9qZ4KLCFj0c7THTga/X/2tIeY9Zdw+OrD1iHnneo6IR7FeW5kz4wGGC5+HHekQylGD TQbwy+Vz 662Fu7vHDZ2vjUSXGjy24mQ6kYowexpl0sPDGbvGxEBvHs5LcuQSovqC9mPjMsnyy/+v4+FDhT1RdTg2jNK9ST1+xJKCkAAVnBeMQF9fRzBLNkq/bXuSrXu2S31CNQNmbT6NdBNt9/vx8mH6NkHmfDC8dO0iBQXPlH9NkU61zR2aJztbwvbC86qrZlfQ4hCkdsl4iWBjTrjmOlUDjVPU7i0Iqti9824YSf3Ub5la3Pra1ylEYtJ8LQ30cPi1fkZHmv2P2vbNjNyPUzjEdtzRmjfrMU7UQjNpws83r2EsjWuoK1mWsyymvWI+xFG13Dr7le80bDaxMYCFPPPTk1hWPkSRA7PbHmGtTbFuqmYaZo4EoUxj8HYc6xEUKkmXlPgixeJogGQSko3TX4iCfONS9rKJ8p38sf6TQ6/gzGjcMRkkUtVx9KP5rLbawq29LjRngOdvaoE+VydggQ/zJFDIob3RQ1AZk9b6t1TLOSSrd8GMwfBo= 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, Apr 17, 2023 at 10:16:28AM -0300, Jason Gunthorpe wrote: > On Mon, Apr 17, 2023 at 02:13:39PM +0100, Lorenzo Stoakes wrote: > > On Mon, Apr 17, 2023 at 10:09:36AM -0300, Jason Gunthorpe wrote: > > > On Sat, Apr 15, 2023 at 12:27:31AM +0100, Lorenzo Stoakes wrote: > > > > The only instances of get_user_pages_remote() invocations which used the > > > > vmas parameter were for a single page which can instead simply look up the > > > > VMA directly. In particular:- > > > > > > > > - __update_ref_ctr() looked up the VMA but did nothing with it so we simply > > > > remove it. > > > > > > > > - __access_remote_vm() was already using vma_lookup() when the original > > > > lookup failed so by doing the lookup directly this also de-duplicates the > > > > code. > > > > > > > > This forms part of a broader set of patches intended to eliminate the vmas > > > > parameter altogether. > > > > > > > > Signed-off-by: Lorenzo Stoakes > > > > --- > > > > arch/arm64/kernel/mte.c | 5 +++-- > > > > arch/s390/kvm/interrupt.c | 2 +- > > > > fs/exec.c | 2 +- > > > > include/linux/mm.h | 2 +- > > > > kernel/events/uprobes.c | 10 +++++----- > > > > mm/gup.c | 12 ++++-------- > > > > mm/memory.c | 9 +++++---- > > > > mm/rmap.c | 2 +- > > > > security/tomoyo/domain.c | 2 +- > > > > virt/kvm/async_pf.c | 3 +-- > > > > 10 files changed, 23 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c > > > > index f5bcb0dc6267..74d8d4007dec 100644 > > > > --- a/arch/arm64/kernel/mte.c > > > > +++ b/arch/arm64/kernel/mte.c > > > > @@ -437,8 +437,9 @@ static int __access_remote_tags(struct mm_struct *mm, unsigned long addr, > > > > struct page *page = NULL; > > > > > > > > ret = get_user_pages_remote(mm, addr, 1, gup_flags, &page, > > > > - &vma, NULL); > > > > - if (ret <= 0) > > > > + NULL); > > > > + vma = vma_lookup(mm, addr); > > > > + if (ret <= 0 || !vma) > > > > break; > > > > > > Given the slightly tricky error handling, it would make sense to turn > > > this pattern into a helper function: > > > > > > page = get_single_user_page_locked(mm, addr, gup_flags, &vma); > > > if (IS_ERR(page)) > > > [..] > > > > > > static inline struct page *get_single_user_page_locked(struct mm_struct *mm, > > > unsigned long addr, int gup_flags, struct vm_area_struct **vma) > > > { > > > struct page *page; > > > int ret; > > > > > > ret = get_user_pages_remote(*mm, addr, 1, gup_flags, &page, NULL, NULL); > > > if (ret < 0) > > > return ERR_PTR(ret); > > > if (WARN_ON(ret == 0)) > > > return ERR_PTR(-EINVAL); > > > *vma = vma_lookup(mm, addr); > > > if (WARN_ON(!*vma) { > > > put_user_page(page); > > > return ERR_PTR(-EINVAL); > > > } > > > return page; > > > } > > > > > > It could be its own patch so this change was just a mechanical removal > > > of NULL > > > > > > Jason > > > > > > > Agreed, I think this would work better as a follow up patch however so as > > not to distract too much from the core change. > > I don't think you should open code sketchy error handling in several > places and then clean it up later. Just do it right from the start. > Intent was to do smallest change possible (though through review that grew of course), but I see your point, in this instance this is fiddly stuff and probably better to abstract it to enforce correct handling. I'll respin + add something like this. > Jason