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 39177E95A96 for ; Mon, 9 Oct 2023 15:22:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C05AF900004; Mon, 9 Oct 2023 11:22:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BB5E2900002; Mon, 9 Oct 2023 11:22:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A5663900004; Mon, 9 Oct 2023 11:22:38 -0400 (EDT) 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 96A63900002 for ; Mon, 9 Oct 2023 11:22:38 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 614CE80231 for ; Mon, 9 Oct 2023 15:22:38 +0000 (UTC) X-FDA: 81326289996.17.DBE1E42 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf08.hostedemail.com (Postfix) with ESMTP id 24821160026 for ; Mon, 9 Oct 2023 15:22:35 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=prD+g92O; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=QFDtSTrw; spf=pass (imf08.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696864956; a=rsa-sha256; cv=none; b=O7G5tTvktru6J/6sURq3CKDktgV5RWO7llGPnZXWLX2OmgGIBtzlOKML9uqjhGn+TLDqSF RR+kcmGQA8W7YqKauGGc4FMtYLJEJf2chFWd3LwWNG6xtuqKV5Pw6tvgiZcnP1OnRvcvFC hqaETHRNufWRd8hKfXH/yoBrPw6zD0o= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=prD+g92O; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=QFDtSTrw; spf=pass (imf08.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.29 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696864956; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=eZqJTmyvE7mhkAtJX6OCEVRGyjVQ+wuvXLRQc/KygIM=; b=IFAOyEZ3kwYYA7Q+wXJWDkVqQDZQlZOL6UE7/6unU06X2ce96+qQW/IhxObbqrG2W320Gj WDyfXDGBWeV4+JwLNBinHiqGJM7dk5Pw4tEgiKKCGHG27Iy3kRra1Kt8jlJjtGPiGkNWdp oZgMdtZPK8vh0X5W34pH5JvbCT7uEUo= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id EB1E11F381; Mon, 9 Oct 2023 15:22:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1696864953; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eZqJTmyvE7mhkAtJX6OCEVRGyjVQ+wuvXLRQc/KygIM=; b=prD+g92Oe2E1hoxZeoUq4sNGzLyrkDszUl/+fYGQL/mO/ZQRXFcl+U1jQFvuyok86SUFo5 /PbYJQUR2hD10n1xl7zoCGLi4T+1EA5DOIsuSHpxFeLmIok+4iBrvRTgRz2I+/bZpL/vdT tUx1XcNwQ/k98edF7nDWmJV0zeTDujc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1696864953; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eZqJTmyvE7mhkAtJX6OCEVRGyjVQ+wuvXLRQc/KygIM=; b=QFDtSTrwRWiNzGZUGJoPHpQNX86CdoTFOnGyP+IfqorYVLvC+XzbUrfuFBOuPPKNt/DqUh a63Nu82yuWYme4CA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id C899913586; Mon, 9 Oct 2023 15:22:33 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id yNRaMLkaJGXQLQAAMHmgww (envelope-from ); Mon, 09 Oct 2023 15:22:33 +0000 Message-ID: <6feb6f37-dfb9-0fe1-1303-2744ad2758d9@suse.cz> Date: Mon, 9 Oct 2023 17:22:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 1/4] mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al. Content-Language: en-US To: Lorenzo Stoakes , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Alexander Viro , Christian Brauner Cc: "=Liam R . Howlett" , linux-fsdevel@vger.kernel.org References: From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 24821160026 X-Stat-Signature: 7pq6s86we8qzrh3ydqm8o56q8ig7mhci X-Rspam-User: X-HE-Tag: 1696864955-709001 X-HE-Meta: U2FsdGVkX18Cg3nAgH94EGj2A5QLBHFk4ONAoybezgUHaWaoGbUle19jc61OtH8UOvfOVTTP1/RHqL5wJtVKEtZm5B4MAC+BbWxAGNjHGOOMtv4cyShmcJPuMJccS8pueJjs1C6WcnORcoojEgLB5gL1HrUvGq4xrBMTqQj687s4BWQyvkH8kjPG0Ita7e/7hT4UGzEDf8g0euphGzOVehY/kwpO71szAdCX6/l2uzPYAcv9LJQCT0fhR/fNswtWfrMu7qh1+vUdgyFMRxO8TRf6uQ8JC7LuQZvRF3lVFDsqOk6gbf6JGklAaeLhFJrhTtou0g1JtPY43IcJKMFe0neYzZVN2KOxEddkDrrhQq+024eMT+74qYKADRWI7pRxURD9Olg35Ere9y3mkXnVQDBOR6OYXMc8XpXyQ/0C0z66p4nITwcA0NLADDT2VWYFfhZ0bU0HNXJi6UU2u7tM3CBrbbUstUr73suU1iyy/04FAWsGx52KgvVtwbRuShXxVrC59x8IbAzr2LdHeM+aW40I98ArKTwP8AMS4gRPLb+V6i3W2OUW+qT8ynrIE6uFvCIVEHrLMqThp7J36TpSUh4UuC9lgR3zA+hAIywj9DaO6vz2bWtA5fHLxpKogqTDOdm+8KNn5AwzXmQMuCtFrkesLxQoNiGM59YaVIbkSmSbzIYY8w1otS0fkMxpoMJqd86HeiMG966aYsEhJIvFhEOadkcFC4q4cpD0YZYbvw+0XNVCEKmgq+QOAIM2vzhAgsZ/HjaZQqwdZpmgCs0UJR+xdrjySn/Y0w7qtAHphfUk3Ar/3eSPYBWBXBO415UGvMhOfu4qFsWnpoP1CDOWyQ9Z82lzJFzGVWXzwh+hn1Vaxh3+gwamoQADqw4Mhun3N6RN6CuEjhsM4kWQxhtxU3WE+cI/+8fUfQ7XBR1PLe7E5Km83A/lmufg4UuruzzEkYzNYnCOE3u5gmtTdcF xSVphNJ/ AEBDy/0eW3j5sdiMlWq72qchVNnpNC1WHIsKc/+ZvD7Dkv2C952MPZ6i9ltdmmJHf5VDDLXOsooN7jwMUcyzcLT5sYLX8lfm4m96UNOuhVitzvSzj0FIuKfQ8CfnP8A7pHIFgI3uLs9HbezKvR16aybvk/E5UNkmxKwH/y0g0CwD+HIiVRllUU+kgRbX0KFuOhMOd4keAYI01/fW+3A3iDXG6elSoJU+gU4YqiC2PY9bFsqy//6S8kQKt5Cw69Lsi8NM/B2mllSGPm4A= 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 10/8/23 22:23, Lorenzo Stoakes wrote: > mprotect() and other functions which change VMA parameters over a range > each employ a pattern of:- > > 1. Attempt to merge the range with adjacent VMAs. > 2. If this fails, and the range spans a subset of the VMA, split it > accordingly. > > This is open-coded and duplicated in each case. Also in each case most of > the parameters passed to vma_merge() remain the same. > > Create a new static function, vma_modify(), which abstracts this operation, > accepting only those parameters which can be changed. > > To avoid the mess of invoking each function call with unnecessary > parameters, create wrapper functions for each of the modify operations, > parameterised only by what is required to perform the action. Nice! > Note that the userfaultfd_release() case works even though it does not > split VMAs - since start is set to vma->vm_start and end is set to > vma->vm_end, the split logic does not trigger. > > In addition, since we calculate pgoff to be equal to vma->vm_pgoff + (start > - vma->vm_start) >> PAGE_SHIFT, and start - vma->vm_start will be 0 in this > instance, this invocation will remain unchanged. > > Signed-off-by: Lorenzo Stoakes > --- > fs/userfaultfd.c | 53 +++++++++----------------- > include/linux/mm.h | 23 ++++++++++++ > mm/madvise.c | 25 ++++--------- > mm/mempolicy.c | 20 ++-------- > mm/mlock.c | 24 ++++-------- > mm/mmap.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ > mm/mprotect.c | 27 ++++---------- > 7 files changed, 157 insertions(+), 108 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index a7c6ef764e63..9e5232d23927 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -927,11 +927,10 @@ static int userfaultfd_release(struct inode *inode, struct file *file) > continue; > } > new_flags = vma->vm_flags & ~__VM_UFFD_FLAGS; > - prev = vma_merge(&vmi, mm, prev, vma->vm_start, vma->vm_end, > - new_flags, vma->anon_vma, > - vma->vm_file, vma->vm_pgoff, > - vma_policy(vma), > - NULL_VM_UFFD_CTX, anon_vma_name(vma)); > + prev = vma_modify_uffd(&vmi, prev, vma, vma->vm_start, > + vma->vm_end, new_flags, > + NULL_VM_UFFD_CTX); > + > if (prev) { > vma = prev; > } else { > @@ -1331,7 +1330,6 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, > unsigned long start, end, vma_end; > struct vma_iterator vmi; > bool wp_async = userfaultfd_wp_async_ctx(ctx); > - pgoff_t pgoff; > > user_uffdio_register = (struct uffdio_register __user *) arg; > > @@ -1484,26 +1482,18 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, > vma_end = min(end, vma->vm_end); > > new_flags = (vma->vm_flags & ~__VM_UFFD_FLAGS) | vm_flags; > - pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT); > - prev = vma_merge(&vmi, mm, prev, start, vma_end, new_flags, > - vma->anon_vma, vma->vm_file, pgoff, > - vma_policy(vma), > - ((struct vm_userfaultfd_ctx){ ctx }), > - anon_vma_name(vma)); > + prev = vma_modify_uffd(&vmi, prev, vma, start, vma_end, > + new_flags, > + ((struct vm_userfaultfd_ctx){ ctx })); > if (prev) { This will hit also for IS_ERR(prev), no? > /* vma_merge() invalidated the mas */ > vma = prev; > goto next; > } > - if (vma->vm_start < start) { > - ret = split_vma(&vmi, vma, start, 1); > - if (ret) > - break; > - } > - if (vma->vm_end > end) { > - ret = split_vma(&vmi, vma, end, 0); > - if (ret) > - break; > + > + if (IS_ERR(prev)) { So here's too late to test for it. AFAICS the other usages are like this as well. > diff --git a/include/linux/mm.h b/include/linux/mm.h > index a7b667786cde..c069813f215f 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -3253,6 +3253,29 @@ extern struct vm_area_struct *copy_vma(struct vm_area_struct **, > unsigned long addr, unsigned long len, pgoff_t pgoff, > bool *need_rmap_locks); > extern void exit_mmap(struct mm_struct *); > +struct vm_area_struct *vma_modify_flags(struct vma_iterator *vmi, > + struct vm_area_struct *prev, > + struct vm_area_struct *vma, > + unsigned long start, unsigned long end, > + unsigned long new_flags); > +struct vm_area_struct *vma_modify_flags_name(struct vma_iterator *vmi, > + struct vm_area_struct *prev, > + struct vm_area_struct *vma, > + unsigned long start, > + unsigned long end, > + unsigned long new_flags, > + struct anon_vma_name *new_name); > +struct vm_area_struct *vma_modify_policy(struct vma_iterator *vmi, > + struct vm_area_struct *prev, > + struct vm_area_struct *vma, > + unsigned long start, unsigned long end, > + struct mempolicy *new_pol); > +struct vm_area_struct *vma_modify_uffd(struct vma_iterator *vmi, > + struct vm_area_struct *prev, > + struct vm_area_struct *vma, > + unsigned long start, unsigned long end, > + unsigned long new_flags, > + struct vm_userfaultfd_ctx new_ctx); Could these be instead static inline wrappers, and vma_modify exported instead of static? Maybe we could also move this to mm/internal.h? Which would mean fs/userfaultfd.c would have to start including it, but as it's already so much rooted in mm, it shouldn't be wrong? > > static inline int check_data_rlimit(unsigned long rlim, > unsigned long new, > diff --git a/mm/madvise.c b/mm/madvise.c > index a4a20de50494..73024693d5c8 100644