From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from shelob.surriel.com (shelob.surriel.com [96.67.55.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7D3D42E7398 for ; Mon, 20 Jul 2026 15:24:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=96.67.55.147 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784561095; cv=none; b=u7scKAkl49Blo6wxlZflMBwXR3A+BF0c4xqReYrSVarqS2HDRAOYR9/mWRzRBBGfPz2R4z3C+EEHj0Sw0WOv3lV7ywymyKcI+xlgSBq9shdhQK0Mq18P/QfHjq7PVzLf2fpoIxVdA/grB6Z6C446EJ6RfcAoVIpMg87te2DM1v4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784561095; c=relaxed/simple; bh=8Zf3Z+flS6hGRtCt7XtkuzPbMWS3fGE2Q0sLjyk163g=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=MTJhGHYo5q5A2XrDfAPnhrfajkw1x7FTHEVhgwswC0wLLPVbFbdHFcjKiiVO/Xf29glA42gmMGdT2HbC0kvaDnCNSp/Q7mJtyYCsC9LJsNAddtqgVhavMZ+UAjT1u3ehnd5KPZnPWIXW81YsLdhX56ry/2TShrRtu4kAnf640Rc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=surriel.com; spf=pass smtp.mailfrom=surriel.com; dkim=pass (2048-bit key) header.d=surriel.com header.i=@surriel.com header.b=oq/oQTDt; arc=none smtp.client-ip=96.67.55.147 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=surriel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=surriel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=surriel.com header.i=@surriel.com header.b="oq/oQTDt" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=surriel.com ; s=mail; h=MIME-Version:Content-Transfer-Encoding:Content-Type:References: In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=hEQ9hUpRdURqegaHoig/c4svS6qvm4546G9gvQSz9O0=; b=oq/oQTDtoJjPI02uU1ZIe3VrEL k69uDO1QYL8yWuayI6i1JQfGxFRu9nnkVWIFNiqJ5XOroDmXEMgyNzTXAZql6SDsIi7G2f2JVfUO1 +Cdjs7y0igm6Ux5qXkKVOWXX4ZlzqlUERGbdLzqy42uYcc5exOSnur2zEZOO33qEOiaGdLTsUanDs moQEKPk9/eRAKvMLw/PyA4yEaTOMTtjfNpkTH/6AWaWdsJBD/B0sXQWU/CLtQwjD4STwSaEfQB9fZ wAZT8n/bNuL7qxtf+99ETtG0KFP6AauoqMTEaSo+O8D/veSd3B+h0sXz7AIqqu1M6eoMhuFGw4Df2 yP7/5dxQ==; Received: from fangorn.home.surriel.com ([10.0.13.7]) by shelob.surriel.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.97.1) (envelope-from ) id 1wlprN-000000001d4-07Ri; Mon, 20 Jul 2026 11:24:49 -0400 Message-ID: <61d21f204aedae44b2b5cbe2e7ffb1d72fedc5c6.camel@surriel.com> Subject: Re: [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock From: Rik van Riel To: Usama Arif Cc: linux-kernel@vger.kernel.org, Andrew Morton , kernel-team@meta.com, David Hildenbrand , Lorenzo Stoakes , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , linux-mm@kvack.org, Jason Gunthorpe , John Hubbard , Peter Xu Date: Mon, 20 Jul 2026 11:24:48 -0400 In-Reply-To: <20260720123505.1234897-1-usama.arif@linux.dev> References: <20260720123505.1234897-1-usama.arif@linux.dev> Autocrypt: addr=riel@surriel.com; prefer-encrypt=mutual; keydata=mQENBFIt3aUBCADCK0LicyCYyMa0E1lodCDUBf6G+6C5UXKG1jEYwQu49cc/gUBTTk33A eo2hjn4JinVaPF3zfZprnKMEGGv4dHvEOCPWiNhlz5RtqH3SKJllq2dpeMS9RqbMvDA36rlJIIo47 Z/nl6IA8MDhSqyqdnTY8z7LnQHqq16jAqwo7Ll9qALXz4yG1ZdSCmo80VPetBZZPw7WMjo+1hByv/ lvdFnLfiQ52tayuuC1r9x2qZ/SYWd2M4p/f5CLmvG9UcnkbYFsKWz8bwOBWKg1PQcaYHLx06sHGdY dIDaeVvkIfMFwAprSo5EFU+aes2VB2ZjugOTbkkW2aPSWTRsBhPHhV6dABEBAAG0HlJpayB2YW4gU mllbCA8cmllbEByZWRoYXQuY29tPokBHwQwAQIACQUCW5LcVgIdIAAKCRDOed6ShMTeg05SB/986o gEgdq4byrtaBQKFg5LWfd8e+h+QzLOg/T8mSS3dJzFXe5JBOfvYg7Bj47xXi9I5sM+I9Lu9+1XVb/ r2rGJrU1DwA09TnmyFtK76bgMF0sBEh1ECILYNQTEIemzNFwOWLZZlEhZFRJsZyX+mtEp/WQIygHV WjwuP69VJw+fPQvLOGn4j8W9QXuvhha7u1QJ7mYx4dLGHrZlHdwDsqpvWsW+3rsIqs1BBe5/Itz9o 6y9gLNtQzwmSDioV8KhF85VmYInslhv5tUtMEppfdTLyX4SUKh8ftNIVmH9mXyRCZclSoa6IMd635 Jq1Pj2/Lp64tOzSvN5Y9zaiCc5FucXtB9SaWsgdmFuIFJpZWwgPHJpZWxAc3VycmllbC5jb20+iQE +BBMBAgAoBQJSLd2lAhsjBQkSzAMABgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRDOed6ShMTe g4PpB/0ZivKYFt0LaB22ssWUrBoeNWCP1NY/lkq2QbPhR3agLB7ZXI97PF2z/5QD9Fuy/FD/jddPx KRTvFCtHcEzTOcFjBmf52uqgt3U40H9GM++0IM0yHusd9EzlaWsbp09vsAV2DwdqS69x9RPbvE/Ne fO5subhocH76okcF/aQiQ+oj2j6LJZGBJBVigOHg+4zyzdDgKM+jp0bvDI51KQ4XfxV593OhvkS3z 3FPx0CE7l62WhWrieHyBblqvkTYgJ6dq4bsYpqxxGJOkQ47WpEUx6onH+rImWmPJbSYGhwBzTo0Mm G1Nb1qGPG+mTrSmJjDRxrwf1zjmYqQreWVSFEt26tBpSaWsgdmFuIFJpZWwgPHJpZWxAZmIuY29tP okBPgQTAQIAKAUCW5LbiAIbIwUJEswDAAYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AACgkQznneko TE3oOUEQgAsrGxjTC1bGtZyuvyQPcXclap11Ogib6rQywGYu6/Mnkbd6hbyY3wpdyQii/cas2S44N cQj8HkGv91JLVE24/Wt0gITPCH3rLVJJDGQxprHTVDs1t1RAbsbp0XTksZPCNWDGYIBo2aHDwErhI omYQ0Xluo1WBtH/UmHgirHvclsou1Ks9jyTxiPyUKRfae7GNOFiX99+ZlB27P3t8CjtSO831Ij0Ip QrfooZ21YVlUKw0Wy6Ll8EyefyrEYSh8KTm8dQj4O7xxvdg865TLeLpho5PwDRF+/mR3qi8CdGbkE c4pYZQO8UDXUN4S+pe0aTeTqlYw8rRHWF9TnvtpcNzZw== Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.56.2 (3.56.2-2.fc42) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Mon, 2026-07-20 at 05:35 -0700, Usama Arif wrote: > On Fri, 17 Jul 2026 13:00:34 -0400 Rik van Riel > wrote: >=20 > >=20 > > + /* > > + * Validate the VMA up front, like __get_user_pages(). A > > VM_IO/VM_PFNMAP > > + * VMA is not rejected outright: it can hold COWed pages > > that have a > > + * struct page, so let follow_page_mask() look for one, > > and treat only > > + * its struct-page-less PFNs as unreachable. Any other > > rejection > > + * (secretmem, bad permissions, ...) is final. > > + */ > > + ret =3D check_vma_flags(vma, gup_flags); > > + if (ret && !(vma->vm_flags & (VM_IO | VM_PFNMAP))) >=20 > Do you need to somehow restructure check_vma_flags()? >=20 > The first thing that check_vma_flags() does is: >=20 > if (vm_flags & (VM_IO | VM_PFNMAP)) > return -EFAULT; >=20 > check_vma_flags() returns before evaluating VM_READ, > FOLL_FORCE, FOLL_ANON or the architecture permission check. >=20 > The code then ignores that early error and may return a COWed > page through follow_page_mask(). A non-FOLL_FORCE caller > can therefore read a COWed page from a PROT_NONE PFNMAP VMA. In this code path, if we return an error to __access_remote_vm(), we end up falling back to the ->access() hook, and will end up reading the memory that's "under" that COW, instead of the COWed page that is currently in the process we are accessing. In other words, I think the behavior of this code is the way we want, even though the implementation should be cleaned up. How about we change check_vma_flags() so it ignores specified flags? check_vma_flags(vma, gup_flags, ignore_flags) Then this one call to check_vma_flags can have it ignore VM_IO | VM_PFNMAP, since we want to access the COWed pages in those VMAs in this code path, but fall back to ->access when there are no COW pages. >=20 > > + goto fail; > > + pfnmap =3D ret; > > + > > + for (;;) { > > + if (fatal_signal_pending(current)) { > > + ret =3D -EINTR; > > + goto fail; > > + } > > + cond_resched(); > > + > > + page =3D follow_page_mask(vma, addr, > > + gup_flags | FOLL_TOUCH | > > FOLL_GET, > > + &page_mask); > > + if (!IS_ERR_OR_NULL(page)) > > + return page; >=20 > __get_user_pages() does >=20 > flush_anon_page(vma, subpage, start + j * PAGE_SIZE); > flush_dcache_page(subpage); >=20 > before returning the page. Do you need to that here as well above? I suppose on systems with virtually indexed caches we do need that. Thanks for spotting that! Now I also wonder if I could restructure get_user_page_vma_remote() to do the lookup before the get_user_pages_remote(), and have it then use the new get_user_page_vma() function, to avoid the double VMA lookup (why are we doing that, anyway?) >=20 --=20 All Rights Reversed.