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.6 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 EB145C11D07 for ; Thu, 20 Feb 2020 11:57:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7507A207FD for ; Thu, 20 Feb 2020 11:57:20 +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="mGn6YfF7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7507A207FD 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 C10936B0005; Thu, 20 Feb 2020 06:57:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id BC0F16B0006; Thu, 20 Feb 2020 06:57:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A87176B0007; Thu, 20 Feb 2020 06:57:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0191.hostedemail.com [216.40.44.191]) by kanga.kvack.org (Postfix) with ESMTP id 90D1C6B0005 for ; Thu, 20 Feb 2020 06:57:19 -0500 (EST) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 20E70181AEF1E for ; Thu, 20 Feb 2020 11:57:19 +0000 (UTC) X-FDA: 76510354998.12.song14_79fa9fee9fd2b X-HE-Tag: song14_79fa9fee9fd2b X-Filterd-Recvd-Size: 14466 Received: from mail-lj1-f196.google.com (mail-lj1-f196.google.com [209.85.208.196]) by imf34.hostedemail.com (Postfix) with ESMTP for ; Thu, 20 Feb 2020 11:57:18 +0000 (UTC) Received: by mail-lj1-f196.google.com with SMTP id q23so3933153ljm.4 for ; Thu, 20 Feb 2020 03:57:18 -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=QyvKdU+O0UCRce/PS9aoM2vNJYjhVokBPfYewrwRru8=; b=mGn6YfF7q9rCkiX4+l1Qn47reLmNosU5MhTzgScbW//Soy3bWKmtOX7V9gutVc1Xjt vtmmAl3mJWtFGteKp1UiISF+s0A/vgebPCxHLO1ftvjEXsNyR/wKKOdniGyKaPH1J+HL V3dKZ9b3qOTcv+384c2M0HpryQQaxzi1y0cp6uZKR7eH+8cFCiwbvptvUMMR6Pv7HXRo OCco4uqmg97peuivlM0XdwhXEc1w7LuCF7dRLD91i8ZAOug7t4o92foGAvLWl3e5rgkp VR5xYXP4Y0t4YSjYkYtQXO7CGLEKuHd1BiAnH/ouQyFhyQO0+P/41ZidfWQKYjWbk3dw +rCw== 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=QyvKdU+O0UCRce/PS9aoM2vNJYjhVokBPfYewrwRru8=; b=NtxAtoFlh52PddowRikh/jMXaBpMyDGIeifCfLh0lGn4ePF2EDeEeVsEGGVesLW0Vm wBW9ughJJs+MrYoId5jNpQxLULJ8VvdQGPcbOCFKOAEryideFmNAN9TI+OzGTENZizdV dcHq3Ijo4Fg+39tLY2CXzipmj0oJ0MRJpjCd8ovBXSzKP4puNenSMfM08UszsMJUmeU4 7vr6YGCpZFekJXY9dngkohFPXeTxynfBbG2lH59pLnI3KcTobYtmHMTd6itPFznT5qRk TeOuI/pzgTOpS+uYaWqAxEfXHBGREO74re2fo2OTYQLrvV/Svzd+K2QalraaEweM9IbN Of3Q== X-Gm-Message-State: APjAAAWS5+NQJ1DLvTJexfdJuhHxEU9rfj0fSpF47vSHkpsxUl+lXnLo qhEilCB605FLioBv86zPRW5eow== X-Google-Smtp-Source: APXvYqwmGkVDPC7zOffw90TYoxDszMAB/RcERdKpqgWfQ0l6Dhcqa3L4Lwk2vYKNsZ/h0GUwrS+vKQ== X-Received: by 2002:a2e:e12:: with SMTP id 18mr19231969ljo.123.1582199836383; Thu, 20 Feb 2020 03:57:16 -0800 (PST) Received: from box.localdomain ([86.57.175.117]) by smtp.gmail.com with ESMTPSA id l3sm1687238lja.78.2020.02.20.03.57.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Feb 2020 03:57:15 -0800 (PST) Received: by box.localdomain (Postfix, from userid 1000) id D1208100FBB; Thu, 20 Feb 2020 14:57:44 +0300 (+03) Date: Thu, 20 Feb 2020 14:57:44 +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 , Florian Weimer Subject: Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap(). Message-ID: <20200220115744.ummq6j5ejp5qojic@box> References: <20200218173221.237674-1-bgeffon@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20200218173221.237674-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 Tue, Feb 18, 2020 at 09:32:20AM -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 > v5 -> v6: > - Code cleanup suggested by Kirill. >=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 > --- > include/uapi/linux/mman.h | 5 +- > mm/mremap.c | 103 ++++++++++++++++++++++++++++++-------- > 2 files changed, 85 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..fa27103502c5 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,46 @@ 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 && new_len > old_len) { > + mm->locked_vm +=3D (new_len - old_len) >> PAGE_SHIFT; > + *locked =3D true; > + } > + > + /* We always clear VM_LOCKED[ONFAULT] on the old vma */ > + vma->vm_flags &=3D VM_LOCKED_CLEAR_MASK; > + > + 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; > + } > +out: > mm->hiwater_vm =3D hiwater_vm; > =20 > /* Restore VM_ACCOUNT if one or two pieces of vma left */ > @@ -422,16 +457,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; > - } > - > 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 +484,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 +532,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 +540,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 +569,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) { > + 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 +582,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). > + */ < Sorry for delay. > I have hard time understanding the case when new_len !=3D old_len. Correct me if I'm wrong, but looks like that you change the size of old mapping to be the new_len and then create a new of the same new_len. This doesn't look right to me. In my opinion, MREMAP_DONTUNMAP has to leave the old mapping intact. And create the new mapping adjusted to the new_len. Other option is to force new_len =3D=3D old_len if MREMAP_DONTUNMAP is specified. It would simplify the implementation. And I don't see why anybody would really want anything else. > + 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 +611,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; Not related to the effort directly, but do we really use offset_in_page() as a substitute for IS_ERR() here. That's disgusting. > + > out1: > vm_unacct_memory(charged); > =20 > @@ -609,12 +665,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 +692,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 | 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 +723,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 +773,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