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 1708CCD98C7 for ; Wed, 11 Oct 2023 06:34:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A6A896B0181; Wed, 11 Oct 2023 02:34:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A18746B0183; Wed, 11 Oct 2023 02:34:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8B63A6B0181; Wed, 11 Oct 2023 02:34:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 7822F6B017C for ; Wed, 11 Oct 2023 02:34:53 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 40624B4895 for ; Wed, 11 Oct 2023 06:34:53 +0000 (UTC) X-FDA: 81332217666.06.74C38E9 Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by imf08.hostedemail.com (Postfix) with ESMTP id 796A816000D for ; Wed, 11 Oct 2023 06:34:51 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CpZ7l2OK; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=lstoakes@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1697006091; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=VEaM7V4d54AdkDOOySpRi+Dcj4bMPN5Z5/1QUsaZsRY=; b=nbSqzASj2mQZWFWD3YbPvcq5w9L29eh+EiRDHZoZWdWQ0lsnz1StuFQV4oItny+ve2hFul sGrzNaqWeetY/McGoGUW+1irYaYJB3K5zCqhX0+XcFwQ2cCctZQ20p0zLR5Fp5GarMeZNV mGM0tB6W5KEZE6v+sSF6g99PIvgvOZM= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CpZ7l2OK; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.44 as permitted sender) smtp.mailfrom=lstoakes@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1697006091; a=rsa-sha256; cv=none; b=PQ+6bs6XlCy3pBNuElOamDI9ULJMy40lQcXJTj+yz0RzbJ8ja0nVbkDxfUgypGaIEyJMDu q9iJJvHJnpJjrNbHTRNCDR6rQWlqMAGq5bjfFix5+EE2RYFpkaEWBuSfmE1YrWo+MnmlQ2 NxlP6MfMDy59tSLWr8gdM2TdEhIw2Hw= Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-31f71b25a99so6368043f8f.2 for ; Tue, 10 Oct 2023 23:34:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697006090; x=1697610890; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=VEaM7V4d54AdkDOOySpRi+Dcj4bMPN5Z5/1QUsaZsRY=; b=CpZ7l2OKpYGvdW4BAhz//6FbbdyuOMwpo+/i3/pvMOktgBxbe0iOt4eJggnn5OD8xE YNDu1MRFC5ssLMeZdmAGprarauhuWe/iKhy73CxABQuEIGTr2RBZolQU8Se3o5qiiKF1 RGmjkByzJiqhuHURJwTHlZb+e0HwL606jSzU/wzE7rcfq/CYLGD3VhI56SCLkQ7roqzn 8CAgAuf77YlPDlTWmjkkxxQSbyOk7uwr9qNL7A3HvvEtRt4LXmXxxvWc6nIxNclf7ZWv Br5000XXDgRi751FzkXqberkTYUuYLsJY5R0opjyvNXFzYhusKGy02B8CBwJd3QFnwHG ul3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697006090; x=1697610890; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=VEaM7V4d54AdkDOOySpRi+Dcj4bMPN5Z5/1QUsaZsRY=; b=c5UR+ohHMuOMi1GawV+j4bm/dVQ72p2Q0So0T6YmYIC2+Xc+1crn1pMRBqyCT8K+QY wU0Yj35RVfuDPv44frKB7EQPgajra1dKN1riwbmW5RAEJNvxXoDPZTCsCyKcKxvK6BzE OM5pwqcrQhqgTegxEfhA4Hy6Bw4KY3V9onDEf2Q9KLfKrjBhL47EZvncb38JUs1ow93P Y3oCYOuNuNc1PxQvmvJkL6i7cRZxFz6SkrWv0YzFq7+HmGaU5BlYHSXatdJHB1sg2mZs AJ1rgf4Nu3fKz5r0xwfFIvjmOsO/4aFsJkj8sA8IZzll3gaVstjIvbnddx1umRVqTf7B FY7Q== X-Gm-Message-State: AOJu0YzCV98eSboTmKLISndhGDInrgmD0ZK5Ail4c5x6fkMclhWxwNyp L4ofN6yd0tlc2v7TooBeejk= X-Google-Smtp-Source: AGHT+IEad+JjLcsG7Ju6pbvc8u/EAVo74u0T4ed/MRRah8dUsAVFhxtVbRLp40+jHJwwVqQ9f361Jg== X-Received: by 2002:a5d:5b1b:0:b0:329:6e92:8d73 with SMTP id bx27-20020a5d5b1b000000b003296e928d73mr13728679wrb.67.1697006089625; Tue, 10 Oct 2023 23:34:49 -0700 (PDT) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id a12-20020adff7cc000000b0031423a8f4f7sm14494828wrq.56.2023.10.10.23.34.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 23:34:48 -0700 (PDT) Date: Wed, 11 Oct 2023 07:34:47 +0100 From: Lorenzo Stoakes To: "Liam R. Howlett" , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Alexander Viro , Christian Brauner , Vlastimil Babka , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2 2/5] mm: abstract the vma_merge()/split_vma() pattern for mprotect() et al. Message-ID: <02957e4c-6752-4039-bcc5-c00b971ad0aa@lucifer.local> References: <20231011021452.57vhftchkfxebppe@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231011021452.57vhftchkfxebppe@revolver> X-Rspamd-Queue-Id: 796A816000D X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: nqfus5hzkciwiwxdirrbo4hphg4coima X-HE-Tag: 1697006091-481615 X-HE-Meta: U2FsdGVkX1+IQTnYCu1oiZcj6cgFUrLR4bXZWITBisQWgbOOOfVRKsM5SWv0+7zcVMWmUaQBtPNXFxb3YCV+aXL3xjWNKsF2ujKUSG9YYOBqFjoNtpc4pUSStQefB/5nCTQURyKdXygLlfVm8FYgb1owXRdrM61wMGJgBkdPgDjc0V9dcckZRA085h8kYYcP8+uWOMSbtXKhXr8XVBb1JOHW728fvmXw/E5oJhsMdMLlYcZ7A0wZb0qwkptngKQmjoyNtogqRBcpT2CgeiDM6hrbqnjKyJ6q2u1fuXTEEuAXI3gEjTCFWWlXQbdyAt7e0DXK3DR1q1pVqtM1I4oMCSdHWe18ACLMvxb03rpyC8cNC9tu6EEWZuobJ34xINhoovS3qOkw/5kL2UHSZSX03DEJJUzHNCjrrza/vhzGtZKtnJnJJzhYpo+f9AbHctwC70jWESuJuAdRo9bt4AT2vG6mB8RA9TvgOsnflQR5Cz+BH2X/bzWSLlBL0cTyXuQCkbpDbWOaoI8pfIMdQQi692B4YO1Iv2qKpSf70sj3+Arfvt5AqQtzOqFquc9ZikMrhZVRK18Ht66ciEyBYK7kUQKcIS8v7SA+BGCfI/Fx++3fIRMHCKUXN2bcunz/9LyrLDa5XiUaB8KmVCSy0OfYrRVS+ryH05MtIBKG5PbiS/QB/g7I/69sXVEmHLplWpzJoia66sXbqQjIQlARFHXl+k2hTVolIU9L8eI7obVLhiWdM5X204E4EcSeq2MnRrH4vJzOlAjOp1jWFAiljyn7wkGjA7GW74B1WZnw/lyUwz+wQ8UzqkmiVTZ/J0yut8X7P/46Kdmv5XbAnFle4CzpDZ5Fa8LnJlN6mjK0IzV3uwSC23AFB+eZwmCc0rHSoMvql6aoF/UpmvTlLD6idXlWeyuahMIZFkG7+kPJdZvR0JlLB8VSazZnzqY/LFSRvJsj0jftWFLN0hPQpQ5ZbLx f9vPPf+u VIDMAO6bQaZHc/Rz9c5BcZdxsAd1aM0syUOH5d1isQXpKb+1aMXy0HiUPXKYU2uGPjBujxqTy7ieqntX6hc/AIDhu4ckO2eMyA2/eOuL3wKnKiX4ZwoSMitrLsPwZVY2MI8ek6TK0+lzFqSmd1ydHzVFbfYUQPXmrBMGfgKKoj555oL2OEKHZZihT9gXoYW7oZW0T8LXiL7D0Kn+5qW0l9cu2ZE7mH8wyTcrOuweKNMNINeAgtCZLVkp8bX/yTQ7v6yEx4OBTvqdsaNxySPT87/e6x4ilTjYCe5nq6bXQs7FvkOvDRMnU+VOM/jaszOzEp+IeYgW6ix8BttYVAkEUqL1OiNmOxA+LIlaIfJvVfh4TZ4sPj5pWRtEEd889N1IHJ/VpX2oXFAuigbLhOcdw4mPRT9mA9qHOe8zJ1RO9UguIjmJ6UD4WTLxW5oOnXIQoXb+PpZWFLMVpHlCmZ0fohMXfAw== 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 Tue, Oct 10, 2023 at 10:14:52PM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes [231009 16:53]: > > 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 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 inline wrapper functions for each of the modify > > operations, parameterised only by what is required to perform the action. > > > > 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 | 69 +++++++++++++++------------------------------- > > include/linux/mm.h | 60 ++++++++++++++++++++++++++++++++++++++++ > > mm/madvise.c | 32 ++++++--------------- > > mm/mempolicy.c | 22 +++------------ > > mm/mlock.c | 27 +++++------------- > > mm/mmap.c | 45 ++++++++++++++++++++++++++++++ > > mm/mprotect.c | 35 +++++++---------------- > > 7 files changed, 157 insertions(+), 133 deletions(-) > > > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > > index a7c6ef764e63..ba44a67a0a34 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_flags_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,28 +1482,17 @@ 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)); > > - if (prev) { > > - /* 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; > > + prev = vma_modify_flags_uffd(&vmi, prev, vma, start, vma_end, > > + new_flags, > > + (struct vm_userfaultfd_ctx){ctx}); > > + if (IS_ERR(prev)) { > > + ret = PTR_ERR(prev); > > + break; > > } > > - next: > > + > > + if (prev) > > + vma = prev; /* vma_merge() invalidated the mas */ > > This is a stale comment. The maple state is in the vma iterator, which > is passed through. I missed this on the vma iterator conversion. Ack, this was coincidentally removed in v3 so this is already resolved.