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 X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0798EC2BA83 for ; Fri, 14 Feb 2020 14:28:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A6C9C2187F for ; Fri, 14 Feb 2020 14:28:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=shutemov-name.20150623.gappssmtp.com header.i=@shutemov-name.20150623.gappssmtp.com header.b="nHSn54iz" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A6C9C2187F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shutemov.name Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 494716B062B; Fri, 14 Feb 2020 09:28:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 41D296B062D; Fri, 14 Feb 2020 09:28:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2E5616B062E; Fri, 14 Feb 2020 09:28:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 0B0196B062B for ; Fri, 14 Feb 2020 09:28:38 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id BD034180AD801 for ; Fri, 14 Feb 2020 14:28:37 +0000 (UTC) X-FDA: 76488963474.15.knife54_4b3d4f1bdc24a X-HE-Tag: knife54_4b3d4f1bdc24a X-Filterd-Recvd-Size: 14117 Received: from mail-lj1-f194.google.com (mail-lj1-f194.google.com [209.85.208.194]) by imf39.hostedemail.com (Postfix) with ESMTP for ; Fri, 14 Feb 2020 14:28:36 +0000 (UTC) Received: by mail-lj1-f194.google.com with SMTP id x7so10968280ljc.1 for ; Fri, 14 Feb 2020 06:28:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov-name.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=AafOgySyVAzNy0c2ESmrKX2K4Va55RytPel0n9VaSzQ=; b=nHSn54izT9Msimbur8SPtDE3YxZvL5kxU+xWVBaAscT1fbbIdKw83O2OvwloH0I5n5 QievxaE0p4d/SwFB2vQRyII4ea75N1wGlUVU+Lz/LHfbWA2V1szE4HxJYd2kjC+2BZq8 GG+t4QpWHrn45qA0KMo1bmJh8LLBejR1KrE9+Ak9WENSGwHSA2NA/zJDDoqMpXOPKcue 50nqBRxhURgBbPUVFCR8vSVNVBeG1J/qzsnprAYXmyUdSn/po4s8AuzezOSXe/1uresb WaIEZfm02srJijxWKIsw4BlIreBDeCVfxlU+6Y8EiX6SDr9MQAMucUSzX4j68bXil+Cm bSxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=AafOgySyVAzNy0c2ESmrKX2K4Va55RytPel0n9VaSzQ=; b=PyNA4gb6X90x2pyH4mtGIvcqbRPJE6WnAjG8LxCZXGqZWld7f6ajq/vqkiQWWyIaRd RUCpZxz15awlhKBqoNuN/EYcjHGvgS4jO7hTz32BwtRMjwyLnvn59TXIFcI05/9OyejW /or++qYuUXivXqo3ivmT9yl4usCg9WelCNEz5BMerAy850XaHGecXBPUzRWOxRuPUo/d opQzaQ1zOSK6qjNYr1Ld1l1fYq3VbommT5CjDwGRFKWZRCk52b9oHTcxw0hWixE6eiwD PTunca1ZYdu3SrcN0imOwuynOe4vC0U3TfWpMJySjUx6LnF/XVu/HE7PRSZzRF7UNO6D PR6g== X-Gm-Message-State: APjAAAXe3Zgula+JRDYymr58gXAbB7ndtQKy1hf8PtQPTt92rYOEeU5E QWgiL0tCJcEinM8+VgX+WOSbCA== X-Google-Smtp-Source: APXvYqyNgbOD18Nh05jEbTbR/WGlDFR4IPYAZjI0gDO74z2U9weVu95HKeKUBL3KbdshfB+UFiX7cw== X-Received: by 2002:a2e:860c:: with SMTP id a12mr2344290lji.146.1581690515304; Fri, 14 Feb 2020 06:28:35 -0800 (PST) Received: from box.localdomain ([86.57.175.117]) by smtp.gmail.com with ESMTPSA id i1sm3594789lji.71.2020.02.14.06.28.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Feb 2020 06:28:34 -0800 (PST) Received: by box.localdomain (Postfix, from userid 1000) id 37EF1100F30; Fri, 14 Feb 2020 17:28:57 +0300 (+03) Date: Fri, 14 Feb 2020 17:28:57 +0300 From: "Kirill A. Shutemov" To: Brian Geffon Cc: Andrew Morton , "Michael S . Tsirkin" , Arnd Bergmann , linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-api@vger.kernel.org, Andy Lutomirski , Will Deacon , Andrea Arcangeli , Sonny Rao , Minchan Kim , Joel Fernandes , Yu Zhao , Jesse Barnes , Nathan Chancellor , Florian Weimer Subject: Re: [PATCH v5 1/2] mm: Add MREMAP_DONTUNMAP to mremap(). Message-ID: <20200214142857.kcmjiequhfl3sot2@box> References: <20200207201856.46070-1-bgeffon@google.com> <20200214040952.43195-1-bgeffon@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20200214040952.43195-1-bgeffon@google.com> Content-Transfer-Encoding: quoted-printable 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, Feb 13, 2020 at 08:09:51PM -0800, Brian Geffon wrote: > When remapping an anonymous, private mapping, if MREMAP_DONTUNMAP is > set, the source mapping will not be removed. The remap operation > will be performed as it would have been normally by moving over the > page tables to the new mapping. The old vma will have any locked > flags cleared, have no pagetables, and any userfaultfds that were > watching that range will continue watching it. >=20 > For a mapping that is shared or not anonymous, MREMAP_DONTUNMAP will ca= use > the mremap() call to fail. Because MREMAP_DONTUNMAP always results in m= oving > a VMA you MUST use the MREMAP_MAYMOVE flag. The final result is two > equally sized VMAs where the destination contains the PTEs of the sourc= e. >=20 > We hope to use this in Chrome OS where with userfaultfd we could write > an anonymous mapping to disk without having to STOP the process or worr= y > about VMA permission changes. >=20 > This feature also has a use case in Android, Lokesh Gidra has said > that "As part of using userfaultfd for GC, We'll have to move the physi= cal > pages of the java heap to a separate location. For this purpose mremap > will be used. Without the MREMAP_DONTUNMAP flag, when I mremap the java > heap, its virtual mapping will be removed as well. Therefore, we'll > require performing mmap immediately after. This is not only time consum= ing > but also opens a time window where a native thread may call mmap and > reserve the java heap's address range for its own usage. This flag > solves the problem." >=20 > v4 -> v5: > - Correct commit message to more accurately reflect the behavior. > - Clear VM_LOCKED and VM_LOCKEDONFAULT on the old vma. > =A0 =A0 > Signed-off-by: Brian Geffon >=20 > --- > include/uapi/linux/mman.h | 5 +- > mm/mremap.c | 106 ++++++++++++++++++++++++++++++-------- > 2 files changed, 88 insertions(+), 23 deletions(-) >=20 > diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h > index fc1a64c3447b..923cc162609c 100644 > --- a/include/uapi/linux/mman.h > +++ b/include/uapi/linux/mman.h > @@ -5,8 +5,9 @@ > #include > #include > =20 > -#define MREMAP_MAYMOVE 1 > -#define MREMAP_FIXED 2 > +#define MREMAP_MAYMOVE 1 > +#define MREMAP_FIXED 2 > +#define MREMAP_DONTUNMAP 4 > =20 > #define OVERCOMMIT_GUESS 0 > #define OVERCOMMIT_ALWAYS 1 > diff --git a/mm/mremap.c b/mm/mremap.c > index 1fc8a29fbe3f..a2a792fdbc64 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -318,8 +318,8 @@ unsigned long move_page_tables(struct vm_area_struc= t *vma, > static unsigned long move_vma(struct vm_area_struct *vma, > unsigned long old_addr, unsigned long old_len, > unsigned long new_len, unsigned long new_addr, > - bool *locked, struct vm_userfaultfd_ctx *uf, > - struct list_head *uf_unmap) > + bool *locked, unsigned long flags, > + struct vm_userfaultfd_ctx *uf, struct list_head *uf_unmap) > { > struct mm_struct *mm =3D vma->vm_mm; > struct vm_area_struct *new_vma; > @@ -408,11 +408,49 @@ static unsigned long move_vma(struct vm_area_stru= ct *vma, > if (unlikely(vma->vm_flags & VM_PFNMAP)) > untrack_pfn_moved(vma); > =20 > + if (unlikely(!err && (flags & MREMAP_DONTUNMAP))) { > + if (vm_flags & VM_ACCOUNT) { > + /* Always put back VM_ACCOUNT since we won't unmap */ > + vma->vm_flags |=3D VM_ACCOUNT; > + > + vm_acct_memory(vma_pages(new_vma)); > + } > + > + /* > + * locked_vm accounting: if the mapping remained the same size > + * it will have just moved and we don't need to touch locked_vm > + * because we skip the do_unmap. If the mapping shrunk before > + * being moved then the do_unmap on that portion will have > + * adjusted vm_locked. Only if the mapping grows do we need to > + * do something special; the reason is locked_vm only accounts > + * for old_len, but we're now adding new_len - old_len locked > + * bytes to the new mapping. > + */ > + if (vm_flags & VM_LOCKED) { > + /* We always clear VM_LOCKED[ONFAULT] on the old vma */ > + vma->vm_flags &=3D VM_LOCKED_CLEAR_MASK; > + > + if (new_len > old_len) { > + mm->locked_vm +=3D > + (new_len - old_len) >> PAGE_SHIFT; > + *locked =3D true; This level of code indentation suggests that code restructuring is required. > + } > + } > + > + goto out; > + } > + > if (do_munmap(mm, old_addr, old_len, uf_unmap) < 0) { > /* OOM: unable to split vma, just get accounts right */ > vm_unacct_memory(excess >> PAGE_SHIFT); > excess =3D 0; > } > + > + if (vm_flags & VM_LOCKED) { > + mm->locked_vm +=3D new_len >> PAGE_SHIFT; > + *locked =3D true; > + } I don't follow why this is required. > +out: > mm->hiwater_vm =3D hiwater_vm; > =20 > /* Restore VM_ACCOUNT if one or two pieces of vma left */ > @@ -422,16 +460,12 @@ static unsigned long move_vma(struct vm_area_stru= ct *vma, > vma->vm_next->vm_flags |=3D VM_ACCOUNT; > } > =20 > - if (vm_flags & VM_LOCKED) { > - mm->locked_vm +=3D new_len >> PAGE_SHIFT; > - *locked =3D true; > - } > - Ah. You moved this piece. Why? > return new_addr; > } > =20 > static struct vm_area_struct *vma_to_resize(unsigned long addr, > - unsigned long old_len, unsigned long new_len, unsigned long *p) > + unsigned long old_len, unsigned long new_len, unsigned long flags, > + unsigned long *p) > { > struct mm_struct *mm =3D current->mm; > struct vm_area_struct *vma =3D find_vma(mm, addr); > @@ -453,6 +487,10 @@ static struct vm_area_struct *vma_to_resize(unsign= ed long addr, > return ERR_PTR(-EINVAL); > } > =20 > + if (flags & MREMAP_DONTUNMAP && (!vma_is_anonymous(vma) || > + vma->vm_flags & VM_SHARED)) > + return ERR_PTR(-EINVAL); > + > if (is_vm_hugetlb_page(vma)) > return ERR_PTR(-EINVAL); > =20 > @@ -497,7 +535,7 @@ static struct vm_area_struct *vma_to_resize(unsigne= d long addr, > =20 > static unsigned long mremap_to(unsigned long addr, unsigned long old_l= en, > unsigned long new_addr, unsigned long new_len, bool *locked, > - struct vm_userfaultfd_ctx *uf, > + unsigned long flags, struct vm_userfaultfd_ctx *uf, > struct list_head *uf_unmap_early, > struct list_head *uf_unmap) > { > @@ -505,7 +543,7 @@ static unsigned long mremap_to(unsigned long addr, = unsigned long old_len, > struct vm_area_struct *vma; > unsigned long ret =3D -EINVAL; > unsigned long charged =3D 0; > - unsigned long map_flags; > + unsigned long map_flags =3D 0; > =20 > if (offset_in_page(new_addr)) > goto out; > @@ -534,9 +572,11 @@ static unsigned long mremap_to(unsigned long addr,= unsigned long old_len, > if ((mm->map_count + 2) >=3D sysctl_max_map_count - 3) > return -ENOMEM; > =20 > - ret =3D do_munmap(mm, new_addr, new_len, uf_unmap_early); > - if (ret) > - goto out; > + if (flags & MREMAP_FIXED) { I think it has to be if (!(flags & MREMAP_DONTUNMAP)) { No? > + ret =3D do_munmap(mm, new_addr, new_len, uf_unmap_early); > + if (ret) > + goto out; > + } > =20 > if (old_len >=3D new_len) { > ret =3D do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap); > @@ -545,13 +585,26 @@ static unsigned long mremap_to(unsigned long addr= , unsigned long old_len, > old_len =3D new_len; > } > =20 > - vma =3D vma_to_resize(addr, old_len, new_len, &charged); > + vma =3D vma_to_resize(addr, old_len, new_len, flags, &charged); > if (IS_ERR(vma)) { > ret =3D PTR_ERR(vma); > goto out; > } > =20 > - map_flags =3D MAP_FIXED; > + /* > + * MREMAP_DONTUNMAP expands by new_len - (new_len - old_len), we will > + * check that we can expand by new_len and vma_to_resize will handle > + * the vma growing which is (new_len - old_len). > + */ > + if (flags & MREMAP_DONTUNMAP && > + !may_expand_vm(mm, vma->vm_flags, new_len >> PAGE_SHIFT)) { > + ret =3D -ENOMEM; > + goto out; > + } > + > + if (flags & MREMAP_FIXED) > + map_flags |=3D MAP_FIXED; > + > if (vma->vm_flags & VM_MAYSHARE) > map_flags |=3D MAP_SHARED; > =20 > @@ -561,10 +614,16 @@ static unsigned long mremap_to(unsigned long addr= , unsigned long old_len, > if (offset_in_page(ret)) > goto out1; > =20 > - ret =3D move_vma(vma, addr, old_len, new_len, new_addr, locked, uf, > + /* We got a new mapping */ > + if (!(flags & MREMAP_FIXED)) > + new_addr =3D ret; > + > + ret =3D move_vma(vma, addr, old_len, new_len, new_addr, locked, flags= , uf, > uf_unmap); > + > if (!(offset_in_page(ret))) > goto out; > + > out1: > vm_unacct_memory(charged); > =20 > @@ -609,12 +668,16 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsi= gned long, old_len, > addr =3D untagged_addr(addr); > new_addr =3D untagged_addr(new_addr); > =20 > - if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE)) > + if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP)) > return ret; > =20 > if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE)) > return ret; > =20 > + /* MREMAP_DONTUNMAP is always a move */ > + if (flags & MREMAP_DONTUNMAP && !(flags & MREMAP_MAYMOVE)) > + return ret; > + > if (offset_in_page(addr)) > return ret; > =20 > @@ -632,9 +695,10 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsig= ned long, old_len, > if (down_write_killable(¤t->mm->mmap_sem)) > return -EINTR; > =20 > - if (flags & MREMAP_FIXED) { > + if (flags & MREMAP_FIXED || flags & MREMAP_DONTUNMAP) { if (flags & (MREMAP_FIXED | MREMAP_DONTUNMAP)) { > ret =3D mremap_to(addr, old_len, new_addr, new_len, > - &locked, &uf, &uf_unmap_early, &uf_unmap); > + &locked, flags, &uf, &uf_unmap_early, > + &uf_unmap); > goto out; > } > =20 > @@ -662,7 +726,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsign= ed long, old_len, > /* > * Ok, we need to grow.. > */ > - vma =3D vma_to_resize(addr, old_len, new_len, &charged); > + vma =3D vma_to_resize(addr, old_len, new_len, flags, &charged); > if (IS_ERR(vma)) { > ret =3D PTR_ERR(vma); > goto out; > @@ -712,7 +776,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsign= ed long, old_len, > } > =20 > ret =3D move_vma(vma, addr, old_len, new_len, new_addr, > - &locked, &uf, &uf_unmap); > + &locked, flags, &uf, &uf_unmap); > } > out: > if (offset_in_page(ret)) { > --=20 > 2.25.0.265.gbab2e86ba0-goog >=20 --=20 Kirill A. Shutemov