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 9EF96C61DA4 for ; Wed, 15 Mar 2023 22:05:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CA2386B0071; Wed, 15 Mar 2023 18:05:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C04BA6B0072; Wed, 15 Mar 2023 18:05:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A57F96B0075; Wed, 15 Mar 2023 18:05:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 95B366B0071 for ; Wed, 15 Mar 2023 18:05:21 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 556961C6B12 for ; Wed, 15 Mar 2023 22:05:21 +0000 (UTC) X-FDA: 80572514442.06.CFDF747 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by imf29.hostedemail.com (Postfix) with ESMTP id 775F4120021 for ; Wed, 15 Mar 2023 22:05:18 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="Mmo/xsvO"; spf=pass (imf29.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 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=1678917918; 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=O+zYG2Px+01uKGNZzjuZ/JXfH1Fwf89A4Lj08OQ0tek=; b=W0UkMb1hUMnVS3noy216dzJSvdE10aNkfTFyd0t4zdNHlX4FicumeSqO8i8PgQ5QAdcPF3 zOqhRXxAKfdJbWFz6FqndyMdWSfpF1K7LWCGB2WNoTHIzV80BjTQp6r/kG+IzqbLsqfedh JI0uhWMZ1jUC4Eka7uo4ul4H6MMq0nY= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="Mmo/xsvO"; spf=pass (imf29.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 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=1678917918; a=rsa-sha256; cv=none; b=VM720BnkePCgg02xzcEp2SerHei8wfbcziXVhsX8bAtGmO2abzvz4kKaWgsp7IhE3E9ouG SxV/s6j14/QqLZes4LT/O2O2iPctjCayeVLVrYZhXwUXR9Ld5MXwzLoU74zRfQkrC3gxrW BkUhzuZ7IEJ12TpQnlUG0/ZkDnBaQP4= Received: by mail-wm1-f51.google.com with SMTP id k25-20020a7bc419000000b003ed23114fa7so1943480wmi.4 for ; Wed, 15 Mar 2023 15:05:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1678917917; 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=O+zYG2Px+01uKGNZzjuZ/JXfH1Fwf89A4Lj08OQ0tek=; b=Mmo/xsvOa2tW4oyHywucXtb0e9AHmI4JUOPupO9156suHJgnIJimTRDbBBZqOFw+oB iXlDgmv62bRYNJp2GpwAoskSAHdjaylT7SU4LqgezI3CUXIXQl6ht2WTxUYHeaZsrd5H dUVdechpZVadqL0pHVS4R727lrdZRbnNrYGHwbAskFClH8spaGOksKfG7ddIrF4ZzR5y tURXSWCiwiOJAsxHVBObjdAk/K7Z5YyqwiCy2lcS2UoH8L+x2Fu/5lDYWrjFNGxIiHQs SQIF1b5lOZXVTxzmoGRxRsBUVn/3uT2ci0NVpP2q/nho+Tc1X095jC5E7g/U9lDiAE4g YMNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678917917; 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=O+zYG2Px+01uKGNZzjuZ/JXfH1Fwf89A4Lj08OQ0tek=; b=yxS6Lt6GSOSXwXPJlzVhVdDWGk/HlSHwbTpdI6DoAMCaAPaO08pggThWH511MCPbhh LIo//Re2EfHHQrR7CGNhmbHVyue5zd0+lejvsulYRmGsw4zh/DofLDvgh8C/xbUY0ygt 6GxYHh99VKnuvBuUh2/FsW5ujqmO8SGG3KzoN/EMa1X8jsddnVMNZmTXkm6EZNM4M9b1 BDsoufhCP0DxISgXCp7Avoyx/hLHW8txOM0d4LFbVmALhCnNq8b8GSvoI1xJXNdNl6IL 0tpkYi9GgzIHkkiyn/DQg17VnMiMjRDsztib1nxpcwIUyp+xotrQ+dJ2KLH+z7jTP64Q pjGg== X-Gm-Message-State: AO0yUKWa9lTZktFzNDWcwGEepRhbkQDai7NvwmPrdLHobvEPUQObYDgG eydhGkIPUIeDBI39lmnBEL8= X-Google-Smtp-Source: AK7set/sT2+cwYFh0P1OMwcqbbKvk0BMeMOFJyi82OI3UTsrR+YQ9B2KDMk7hxL7GuqMl+UEMbfajQ== X-Received: by 2002:a05:600c:35ce:b0:3e2:1368:e3a0 with SMTP id r14-20020a05600c35ce00b003e21368e3a0mr20212833wmq.18.1678917916978; Wed, 15 Mar 2023 15:05:16 -0700 (PDT) Received: from localhost (host86-146-209-214.range86-146.btcentralplus.com. [86.146.209.214]) by smtp.gmail.com with ESMTPSA id t8-20020a1c7708000000b003ed2276cd0dsm3029854wmi.38.2023.03.15.15.05.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 15:05:15 -0700 (PDT) Date: Wed, 15 Mar 2023 22:05:14 +0000 From: Lorenzo Stoakes To: Vlastimil Babka Cc: Andrew Morton , "Liam R. Howlett" , Matthew Wilcox , linux-mm@kvack.org, linux-kernel@vger.kernel.org, patches@lists.linux.dev, maple-tree@lists.infradead.org Subject: Re: [PATCH 09/10] mm/mmap: start distinguishing if vma can be removed in mergeability test Message-ID: <7e0e33a7-4e9b-4714-8d4b-9487c8d1f799@lucifer.local> References: <20230309111258.24079-1-vbabka@suse.cz> <20230309111258.24079-10-vbabka@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230309111258.24079-10-vbabka@suse.cz> X-Stat-Signature: c9ii88ur5s6fcw5sbbh1qbgfsows44k1 X-Rspam-User: X-Rspamd-Queue-Id: 775F4120021 X-Rspamd-Server: rspam06 X-HE-Tag: 1678917918-82866 X-HE-Meta: U2FsdGVkX18te5+0kHtJpgLJcSmvR/XMEuBI/q/uGscDC9MgKpJT8WXjbHNPlABOirv7xkPTY3nVl9jtbJrYF/thxujOTHhQm2cuQjKKYP699YdAhQSJd5XgzeFwx5gxNzTOhUv3S8gqUsv9rr4gypMVU0TaPFSR63R2F6y6oyPAHY60lg2m1OC5JZjByqIstgrQJZTNwIuYLqMcsFnWU8SUfUjeYIxdIbVvf5q1HAOrf3sMKBl9SWZ0lu3MXVB5WJBLP6hbNe6SmNr6HFNOdRbwuFR0SlVuRlexeHrHAfM/1h3rvLlsxU/v5JCoZI19M6OnQOZooQeBegpfZldMuJWrGO6oEgBXgXzg4CS7MqaloNhs4I7cif0mi7EYtzK02mRkM5UAdoJgdd+4zQo/7+lorq381sMQY5TtjicK64FJp4ENHai2cnEwHyB7S9cywJ5yqB+dYLG45Vo4kwg76HlzYD3j29JGYVw/D9US/mg9EraPFR39Op4Qe9hEy8B9Lf7OHEWCQ9KfwgcCIXU+b/VXVycsIE97bOkUJm039qJdDvJV70+Col+Yjc1MG3o/zL0zlols8+hvRzvq4RvZiFOUGj3q5+MbtmbWwWXRF3JPxap3f2XfzO3D75fFHzB672Zk3XxcZpD0sr/6gkPWtiGsfqZbaErZS8ZK/ZvpkL/bRbn8ehIBendwtVgBpTqMhDmKcrcI5tMa76VMK5CqYA95E8QZF/+1AwiZpjxdcwIs2w7oLfTx0rMmEdkBIR2wy+1sKgQDZPGmi4HufDOgzt+HL6Y6CyZMP6xdMFX4/8LVdCYC4rcvoDYhMGpKhtVIgGR5X8cxDSzLJ1jKaUkPjNPK8IpcSnxxo5UCAQMGDb51jB4nO57zhHCdLh40cS0/gO39UU2vvFvYZnwZcsabQ5Qd3g/4WeyfFd7X9zyJX2HvVQWh1F3gc1tO0AgerC+TyJPApc0jUr4QSfkGeiu PFxr8FzY SoMnxdA12DTdMwd40/eqaNeBPsgBrq/vtjxfQ5qIt4AvIhGGv0qPsD6fxWnyjTAD+nv5VzyoRKkMPQZZyhlksmKq9rVWg+1EiRvbXp5GqsyI47+r7Md1LMf8RLC4/Z4QNCVJE7ILiN5Ebbr8cXcr7/ob2DXv8IV+8GslcQla0liIaUe92mWojStOAE0COzayP06BWVc0+EXgKtqtcZm1F4sf9onCZ37xieTE82ELzlV/XV9VseRbvnQqCgaYOezB0AQCV3GzlhaH+M4C3mra7YQcrVvkdjpdb61lzM34LdJFIybgyQrlJAnt1zsR0Q6IdAPRmDthMmAyEAPzzqt/cEUTS/8on2jUfi/0r9SLRRT9L0bHXe4FhRPw8Ia+KSL+jBnKssknbWA5rp8Ghzx5gO0dfHuYdYe8AGCfhgz5ybG8h3EzVVs/99i2sTw== 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 Thu, Mar 09, 2023 at 12:12:57PM +0100, Vlastimil Babka wrote: > Since pre-git times, is_mergeable_vma() returns false for a vma with > vm_ops->close, so that no owner assumptions are violated in case the vma > is removed as part of the merge. > > This check is currently very conservative and can prevent merging even > situations where vma can't be removed, such as simple expansion of > previous vma, as evidenced by commit d014cd7c1c35 ("mm, mremap: fix > mremap() expanding for vma's with vm_ops->close()") > > In order to allow more merging when appropriate and simplify the code > that was made more complex by commit d014cd7c1c35, start distinguishing > cases where the vma can be really removed, and allow merging with > vm_ops->close otherwise. > > As a first step, add a may_remove_vma parameter to is_mergeable_vma(). > can_vma_merge_before() sets it to true, because when called from > vma_merge(), a removal of the vma is possible. > > In can_vma_merge_after(), pass the parameter as false, because no > removal can occur in each of its callers: > - vma_merge() calls it on the 'prev' vma, which is never removed > - mmap_region() and do_brk_flags() call it to determine if it can expand > a vma, which is not removed > > As a result, vma's with vm_ops->close may now merge with compatible > ranges in more situations than previously. We can also revert commit > d014cd7c1c35 as the next step to simplify mremap code again. > > Signed-off-by: Vlastimil Babka > --- > mm/mmap.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index d20bbe9ec613..65503ea07f32 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -742,12 +742,13 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_area_struct *vma, > > /* > * If the vma has a ->close operation then the driver probably needs to release > - * per-vma resources, so we don't attempt to merge those. > + * per-vma resources, so we don't attempt to merge those in case the caller > + * indicates the current vma may be removed as part of the merge. Nit: 'in case the caller indicates' -> 'if the caller indicates' > */ > static inline bool is_mergeable_vma(struct vm_area_struct *vma, > struct file *file, unsigned long vm_flags, > struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > - struct anon_vma_name *anon_name) > + struct anon_vma_name *anon_name, bool may_remove_vma) > { > /* > * VM_SOFTDIRTY should not prevent from VMA merging, if we > @@ -761,7 +762,7 @@ static inline bool is_mergeable_vma(struct vm_area_struct *vma, > return false; > if (vma->vm_file != file) > return false; > - if (vma->vm_ops && vma->vm_ops->close) > + if (may_remove_vma && vma->vm_ops && vma->vm_ops->close) > return false; > if (!is_mergeable_vm_userfaultfd_ctx(vma, vm_userfaultfd_ctx)) > return false; > @@ -793,6 +794,8 @@ static inline bool is_mergeable_anon_vma(struct anon_vma *anon_vma1, > * We don't check here for the merged mmap wrapping around the end of pagecache > * indices (16TB on ia32) because do_mmap() does not permit mmap's which > * wrap, nor mmaps which cover the final page at index -1UL. > + * > + * We assume the vma may be removed as part of the merge. > */ > static bool > can_vma_merge_before(struct vm_area_struct *vma, unsigned long vm_flags, > @@ -800,7 +803,7 @@ can_vma_merge_before(struct vm_area_struct *vma, unsigned long vm_flags, > pgoff_t vm_pgoff, struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > struct anon_vma_name *anon_name) > { > - if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name) && > + if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name, true) && > is_mergeable_anon_vma(anon_vma, vma->anon_vma, vma)) { > if (vma->vm_pgoff == vm_pgoff) > return true; > @@ -814,6 +817,8 @@ can_vma_merge_before(struct vm_area_struct *vma, unsigned long vm_flags, > * > * We cannot merge two vmas if they have differently assigned (non-NULL) > * anon_vmas, nor if same anon_vma is assigned but offsets incompatible. > + * > + * We assume that vma is not removed as part of the merge. > */ > static bool > can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > @@ -821,7 +826,7 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > pgoff_t vm_pgoff, struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > struct anon_vma_name *anon_name) > { > - if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name) && > + if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name, false) && > is_mergeable_anon_vma(anon_vma, vma->anon_vma, vma)) { > pgoff_t vm_pglen; > vm_pglen = vma_pages(vma); > -- > 2.39.2 > I wonder whether this might be moved later into the actual vma_merge() logic so we only abort a merge at the point a VMA with ->close() would otherwise be removed? But obviously that would probably need some further clean up to make it not add yet more complexity to an already very complicated bit of logic. Otherwise, very nice, clear + conservative so, Reviewed-By: Lorenzo Stoakes