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 1A430CD98F5 for ; Wed, 11 Oct 2023 06:58:27 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 966198D00C3; Wed, 11 Oct 2023 02:58:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 915258D0002; Wed, 11 Oct 2023 02:58:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7DD968D00C3; Wed, 11 Oct 2023 02:58:26 -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 6AB348D0002 for ; Wed, 11 Oct 2023 02:58:26 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 2938DC019A for ; Wed, 11 Oct 2023 06:58:26 +0000 (UTC) X-FDA: 81332277012.19.F198740 Received: from mail-wr1-f42.google.com (mail-wr1-f42.google.com [209.85.221.42]) by imf05.hostedemail.com (Postfix) with ESMTP id 5AD9F100005 for ; Wed, 11 Oct 2023 06:58:24 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=hoBq0f3U; spf=pass (imf05.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.42 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=1697007504; a=rsa-sha256; cv=none; b=aeyVrPyJW12dnrGNVOeh0qmF/eVl2GEA3IuhFb1xcCKoVk/8bS983BS9e/Hpd9MYQAg7CF zlzSTTptJAoyKGX3iMIGQI0rcAWCEiJHX8kRfU6+z2HBeKFpTkyOxrUOC0GorPZGXPNWXO mIg0vwA1mEoUwWWWqjo4V9uA5Uj8TCQ= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=hoBq0f3U; spf=pass (imf05.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.42 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=1697007504; 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=94Aupc/WoctqDp8xqPo3RjM/d0ECIJ8ZPHye9gY433w=; b=OEB8VXrOTsQOWyTzVMU+XvwC6+NFgtLQeo7tIa3vLS0wgApFOAEqmJaZmDFJY4LhM3BFC8 NBsYn2Kk53/oUTjvc5uwugMLKVhUjoT7zT0oaD01oTw+Hz2j2LEIFqtDB6hHHL2hhMw7pe wFEIvuxYH3AtO4mdoGFqOcTZPNELnBQ= Received: by mail-wr1-f42.google.com with SMTP id ffacd0b85a97d-307d20548adso5897114f8f.0 for ; Tue, 10 Oct 2023 23:58:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1697007502; x=1697612302; darn=kvack.org; 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=94Aupc/WoctqDp8xqPo3RjM/d0ECIJ8ZPHye9gY433w=; b=hoBq0f3UMp+5zNkowXt/LSMp2kpUUz4d9c7CBXSilFit/7wIdeHIV4LwGs/MS2ue0A Dz1FDh7kfoLtBX6MVK4srYQW+BdPHDVQWe2nN6EVwnYQSilrIzsrBCdlRI89zjXONm+3 MBj4es39aj0Cbs9w/M0lCc6LWGK+gZUW6//wZT6Npaq+pByKLLxw7P2ONe+34B1XabFS ePzijj4goRfy5Pglq8oIPZfsc6dzNP28kWx/MPe/ZaFJLO+bxOQ4q8qfEsDLol0RaBkd z24nWDBe1IKdvUobQHRwKt83uP+OP0KnEGLnhxgmiWcJN71FiN8nb4vb5lU/Eqd1jdF8 PqWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1697007502; x=1697612302; 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=94Aupc/WoctqDp8xqPo3RjM/d0ECIJ8ZPHye9gY433w=; b=hHBKbK+f/0lxWrBlILw6f3N/8LLoJM13Gvxr/9+KdBv/IkN9KpTn5at6uJvQnXaZcC SBhz6uQJRznCoxlS3Ux3Hb+uvS32oa0PrbrjyDzva8OJSEc5zzS1LesusSH+IAPqSmyX 93ioe/JCJINLBNiBY+PU/ug2ODWpcAkVNZSzEPcvDGHSr6Mq6pENjDQ7M8Xm0BCfqYlp 4UTd7SOVjJXfR2tHNop+hwHyv78HXE50uGIz1R9PWxNujGWevVxalEaxRF1ricMAxzpn eJnVws0cM1tkjWUgSPXqLPZmOoHS+pc6ioQOR5mCyScMci7aodw2D3s8LpbKKLE+/HwW 3hEw== X-Gm-Message-State: AOJu0YzDA8S5Hu56xGipobTmHsFLJxZ56SIieWv7tpHPMfdSFOmXdAyf TJUabRMIujmXQeFQ4Wh0ONgNR9BZY68= X-Google-Smtp-Source: AGHT+IHLJYkdW/fjT6jzHgUoZyb7XD0MWnrvSt2FIGvXP4SKbU60fDv2wfSA0vlHElTRAdHzqaci8Q== X-Received: by 2002:a5d:5304:0:b0:319:6d03:13ae with SMTP id e4-20020a5d5304000000b003196d0313aemr16290191wrv.55.1697007502350; Tue, 10 Oct 2023 23:58:22 -0700 (PDT) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id p13-20020a5d68cd000000b0031ae8d86af4sm14469751wrw.103.2023.10.10.23.58.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 23:58:20 -0700 (PDT) Date: Wed, 11 Oct 2023 07:58:20 +0100 From: Lorenzo Stoakes To: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Alexander Viro , Christian Brauner Cc: "=Liam R . Howlett" , Vlastimil Babka , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v3 4/5] mm: abstract merge for new VMAs into vma_merge_new_vma() Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 5AD9F100005 X-Stat-Signature: uxzyx9e1krztr4qx46bn3unqpmnsbm78 X-Rspam-User: X-HE-Tag: 1697007504-279072 X-HE-Meta: U2FsdGVkX1+1ouZs+tpAim/CiLKF4pV9ED/rxSsDOwGM3sVfiB48z3vu4AnD0UVLsfW5Den9yMDn/wLwR3FuY9q2lWrMIcThd8zD2pWK3kpLf7gbwRTmmrgelAlTCUI2JhnaCQI0YdjmZzFXNIZWtntRtHpF6ozL1w01y58rbZ8Qitvn3kHmA6s0Vrb8WOX4la8j1uOHtqhT/X8gJIcgMqGgosPPWgpFahMNcwvBK4a/ScNSAMkPFJA2frFogVZIIQpXQvvMdEyZPMKwcYrExx6CrUeEeHieySKxvZ6knVtXu+AD3q8CpK1638+1Fjw6zMPO5Dl4vSM8wDyUf/tV/TaMhvYKRf1P6M+TC2N2aHR4RXsZO6gkqU05TVol6r/3dtQawW0vtvcCjFks2acpz2NcvfV5TRq7cgd73ad4y7BMPr/aPolwAHhCznqAy/kNdzoOx6iJEbFSbQJV+wl70/ZFLR4NQ6IxNrINNLXQvLAyV5YjgQ7ATMRj2G6Ac06OBFGsr8TzVdZeOqFogHRIa7eNriyRMZh1+jSmXyYULoo7yRUuNmJibSnPqjck0MvTG+DQjUmxtMt/dUH3YratxtErp0BabcuE6St3OW2D8B/FBw0mk+wUnCVIubAn+k4YK6GjhTNW6pLHHX/VzCQ1AGqOpvUTM7Fhl7VR+g6F7e1gNrgfQitzq09rTvL66zrw/vyIKVZG0dssFICZXCxdpS6yAr4o/BqTVcYkD4zlwMP2KOX7G/1QzACLvqAlOk0C3Rp1kOyPOCA+OfIqQIHfjRRZ5s3gBp3xO7IK2feKr75YzAjdBHDGnxkfz0UzujQPRITfPVxF8X7qxi1dr8duAXWasUkq1/KRMYDTbfkvtyrLSna9s8+38E6An3tvMBi8w9a4Ph/UNFAz5TNHVMwo9QPc+Yb8Ndj9CGzrIH0/W2hLkV7OfcOgwLlOarTYYRXCsExvy75TEUAwCsxIPcp 7LKgWD41 97qPanT5DS35ajp+eaWKT+hML5mQmwNM7ZCNwa93EgvdLp+XGsD9i3QskLgn8nXza7jg3tPy8iRKqOH8i8fJ1ssLz5XZvma6SYO3spzn3YT2GUIxQeTPvRazsKs7jxF6Yvtz/2Mr7iEN1mZ5bKpt+L4IMbx7XS6jRGihKfjtEdMKimvZGyC+aS8iwbDp5NqjGqs/YuL9B73POo90XB25A8O3KTtcsVzCQ8YulwmfDHE1tRt7HwFH9Ll3nZYtSf0ier7X0DslPs5KGYeHW0xAOars+icnDFYs3QxKsNMggf8cMWSvjpXvnhAZ+jCemh9Nk1WDewxUS7m3cqUgM3BWvLRrp30rqUkdcho7djKrR9BDsOwDhqZhsKdXdNO7Ecr9xjmO1V5IHX96890FNKbNEr42anvLleecOYoBXADKuvPIZvyugzglANJlLR1YxjtOPovfk 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 07:23:07PM +0100, Lorenzo Stoakes wrote: > Only in mmap_region() and copy_vma() do we attempt to merge VMAs which > occupy entirely new regions of virtual memory. > > We can abstract this logic and make the intent of this invocations of it > completely explicit, rather than invoking vma_merge() with an inscrutable > wall of parameters. > > This also paves the way for a simplification of the core vma_merge() > implementation, as we seek to make it entirely an implementation detail. > > Note that on mmap_region(), VMA fields are initialised to zero, so we can > simply reference these rather than explicitly specifying NULL. Andrew - based on feedback from Liam on the v2 version of this patch, could we change this commit message to:- Only in mmap_region() and copy_vma() do we attempt to merge VMAs which occupy entirely new regions of virtual memory. We can abstract this logic and make the intent of this invocations of it completely explicit, rather than invoking vma_merge() with an inscrutable wall of parameters. This also paves the way for a simplification of the core vma_merge() implementation, as we seek to make it entirely an implementation detail. The VMA merge call in mmap_region() occurs only for file-backed mappings, where each of the parameters previously specified as NULL are defaulted to NULL in vma_init() (called by vm_area_alloc()). This matches the previous behaviour of specifying NULL for a number of fields, however note that prior to this call we pass the VMA to the file system driver via call_mmap(), which may in theory adjust fields that we pass in to vma_merge_new_vma(). Therefore we actually resolve an oversight here by allowing for the fact that the driver may have done this. > > Reviewed-by: Vlastimil Babka > Signed-off-by: Lorenzo Stoakes > --- > mm/mmap.c | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > Thanks! > diff --git a/mm/mmap.c b/mm/mmap.c > index a516f2412f79..db3842601a88 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -2485,6 +2485,22 @@ struct vm_area_struct *vma_modify(struct vma_iterator *vmi, > return vma; > } > > +/* > + * Attempt to merge a newly mapped VMA with those adjacent to it. The caller > + * must ensure that [start, end) does not overlap any existing VMA. > + */ > +static struct vm_area_struct *vma_merge_new_vma(struct vma_iterator *vmi, > + struct vm_area_struct *prev, > + struct vm_area_struct *vma, > + unsigned long start, > + unsigned long end, > + pgoff_t pgoff) > +{ > + return vma_merge(vmi, vma->vm_mm, prev, start, end, vma->vm_flags, > + vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), > + vma->vm_userfaultfd_ctx, anon_vma_name(vma)); > +} > + > /* > * do_vmi_align_munmap() - munmap the aligned region from @start to @end. > * @vmi: The vma iterator > @@ -2840,10 +2856,9 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > * vma again as we may succeed this time. > */ > if (unlikely(vm_flags != vma->vm_flags && prev)) { > - merge = vma_merge(&vmi, mm, prev, vma->vm_start, > - vma->vm_end, vma->vm_flags, NULL, > - vma->vm_file, vma->vm_pgoff, NULL, > - NULL_VM_UFFD_CTX, NULL); > + merge = vma_merge_new_vma(&vmi, prev, vma, > + vma->vm_start, vma->vm_end, > + pgoff); > if (merge) { > /* > * ->mmap() can change vma->vm_file and fput > @@ -3385,9 +3400,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, > if (new_vma && new_vma->vm_start < addr + len) > return NULL; /* should never get here */ > > - new_vma = vma_merge(&vmi, mm, prev, addr, addr + len, vma->vm_flags, > - vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), > - vma->vm_userfaultfd_ctx, anon_vma_name(vma)); > + new_vma = vma_merge_new_vma(&vmi, prev, vma, addr, addr + len, pgoff); > if (new_vma) { > /* > * Source vma may have been merged into new_vma > -- > 2.42.0 >