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 B76B6EB64D7 for ; Wed, 28 Jun 2023 07:24:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 239A78D0003; Wed, 28 Jun 2023 03:24:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1EA358D0001; Wed, 28 Jun 2023 03:24:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0B33A8D0003; Wed, 28 Jun 2023 03:24:44 -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 ECB1D8D0001 for ; Wed, 28 Jun 2023 03:24:43 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C1E9B40854 for ; Wed, 28 Jun 2023 07:24:43 +0000 (UTC) X-FDA: 80951319246.15.F9ADEDE Received: from mail-yb1-f180.google.com (mail-yb1-f180.google.com [209.85.219.180]) by imf26.hostedemail.com (Postfix) with ESMTP id E12EA140018 for ; Wed, 28 Jun 2023 07:24:41 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=33Z37jwY; spf=pass (imf26.hostedemail.com: domain of surenb@google.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687937081; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=YPt1NG5h3/g4q0dxyhrEkAT+vQ/YVJq6MYkZgQvxaTs=; b=kGDZIrJ4xzn/2gjjvLm5ll7zzv7WL7bkE/q1r/BRtIh56dUCik5aHt8w7kxsFew2U+841x ObKoziz4hmGzXH4ga/7aLSkQtW9L3tJtNu8hy5T5W2njHKhL9zZ9+XynPRqVCV7VOI9aCF wvow7pLtiMHKp2IjORpFEjTQQUVdU4Q= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=33Z37jwY; spf=pass (imf26.hostedemail.com: domain of surenb@google.com designates 209.85.219.180 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687937081; a=rsa-sha256; cv=none; b=eBurk//xOr/9QgnRoSSIRbMwlEYTvKNddPzHd5DLEj+y6iNn9PFaalBde5AQzZNaa37odL mQ44B07PgbzTgRSIr9SfOoYpOfayg46cZz8dG+MBz64l+tA2F7NO6CWULgGKeL9ChdHHjO /tkvM+IfEL7zrgDi+DBfRssBW59ZY3c= Received: by mail-yb1-f180.google.com with SMTP id 3f1490d57ef6-be30cbe88b3so5269571276.1 for ; Wed, 28 Jun 2023 00:24:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687937081; x=1690529081; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YPt1NG5h3/g4q0dxyhrEkAT+vQ/YVJq6MYkZgQvxaTs=; b=33Z37jwYvVXmvum6yphRZJcHgE4aQAi4g1gcpcg1AiBAmrt9e7IlWSDceqRIxZbFhA 5yfUT7zytJ35OBZ+LBKNoUGT7ECiH2axs9wzOgonfQAtWoDcLhgtdJxapJ2naZ77UR5s eAI4aNslUXAzYaOszLnvrV7K2kl90N0W+oRk7NPSupmajCW3rGmFfdu9rd4qANnAIlfX Ymq8jxmTkkVr1K/PWBAItdkIhRoL4WPgmty5lwmS4tJAaScJFnCuL7lduR0PGiJW34EH yfHJUj69fuscKcWTQAgvwnNsbaDA32WInBivjb+4UcYGkhPNsEnIzNDy0N6q+XWoHA2w yIIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687937081; x=1690529081; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YPt1NG5h3/g4q0dxyhrEkAT+vQ/YVJq6MYkZgQvxaTs=; b=XQQhvWGcSFWzaUUPirS0I+CUSQBZB5XE3xufSXqJM6oebOg9iS2kKGSa0U5ny1XSaP NjXSCH6Nu3JwjbU8ODeZIhQZaAiItdNWj5gICrJ78cwjdtyzBhWdORz9JQ5NQWEtF8WQ +tMI6MzLFIN6Yhf98XUK+jocMIdLv3bfgtaskfRoJ+VRO0B8tfo/IBjzCrwHI3AXdMMc h8m0sGby6zh31pbmVeI/CkASeeodaWsaUi9TNCdqOvTDpsMctXi/AsnPiMZYha2RBuCq RBW25K9JmHc08pIERafDoGbDXokzA2ZQuPuLMfm4i8XXcZkxwZMWaC/PYXT2KwA45N/z h80A== X-Gm-Message-State: AC+VfDxRIySvQThdQWf/db1RrSQO51pkZ8cKwh+0iuoB09usucCvveKf D8tAOSr/jHnxYDDAfgJvF0ESG3K3BQ5JKVwG1Kp3GQ== X-Google-Smtp-Source: ACHHUZ4leSrybvwaB3IOYDT+lfPJcxVAhx3qlvmG3efbxkO2c6Mk2IatcNGR42NNpsEIteYjfwICI23c7ln4Z8Y7X1U= X-Received: by 2002:a25:42:0:b0:ba8:7e23:f069 with SMTP id 63-20020a250042000000b00ba87e23f069mr30494086yba.44.1687937080731; Wed, 28 Jun 2023 00:24:40 -0700 (PDT) MIME-Version: 1.0 References: <20230628071800.544800-1-surenb@google.com> <20230628071800.544800-7-surenb@google.com> In-Reply-To: <20230628071800.544800-7-surenb@google.com> From: Suren Baghdasaryan Date: Wed, 28 Jun 2023 00:24:29 -0700 Message-ID: Subject: Re: [PATCH v4 6/6] mm: handle userfaults under VMA lock To: akpm@linux-foundation.org Cc: willy@infradead.org, hannes@cmpxchg.org, mhocko@suse.com, josef@toxicpanda.com, jack@suse.cz, ldufour@linux.ibm.com, laurent.dufour@fr.ibm.com, michel@lespinasse.org, liam.howlett@oracle.com, jglisse@google.com, vbabka@suse.cz, minchan@google.com, dave@stgolabs.net, punit.agrawal@bytedance.com, lstoakes@gmail.com, hdanton@sina.com, apopple@nvidia.com, peterx@redhat.com, ying.huang@intel.com, david@redhat.com, yuzhao@google.com, dhowells@redhat.com, hughd@google.com, viro@zeniv.linux.org.uk, brauner@kernel.org, pasha.tatashin@soleen.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: E12EA140018 X-Rspam-User: X-Stat-Signature: qyxj366jjbtf7i56fs86beijgf11x341 X-Rspamd-Server: rspam01 X-HE-Tag: 1687937081-528252 X-HE-Meta: U2FsdGVkX18W63WizAKUBW950al/42SCKDp3Qku62Y8XHOGd0SSCTiunk37fjaHZ57/VSoGtmL2sHZRiHhWYip79R0OwIKkGnVWabL+y/hOs/R1tVPAr5NQ7j4nClzSOynhnQkAzhI4LYS6B/WUcs4SP9w58ooUJmBqnEUYDH0AJYLXzU6WRjPkCQTnMuxsP76evTIFDShTB60so/JKJNZIxug0pPxWN2pEAI22PCmIZu6HtzIauFt9YjIt1UeXvNp0BXRiIOQd4lcvG/YX6fey28iKTc/G4scBhSGzJIIs6Reck7b3Qt++3kCSsyLl3ASkLSIFhhKDy59QqW4dG8iAri3ke3RP24rf7i1ld150ygiDQxi0ciIP8YVzYXnf87sbcHpAke+lrEZmE3UFuDdIwLgqImuJVnlhX1HsND2yQ3okbZuFjJz3BetvX9QcYRnUSHUFU7kpzBHjHOJz2cHzVMFY4iig9F6G1hCuGH8eLFZvoKHVZbLNQsHs7Zo60tky5/gS+NWqYbFZNlAsFTyMmDDnYlBw97KccboXrdXuTNduhJsaeBjhjE4uirLZ6rUzC7ulPzb9gGA4Rh05ekM7KdUo51GKeaZtPc4CDstcU95RUKmhiUfr3ThY1fXwBIMPryZQmABq0/d37zU8clsb9108FQ4EMMBOUokL2n44zIjb5ZWSdwUfwoAEG+7Wq2O4d7DKZp7b2SbTWc4lPmohHUuNb61me7f3aP9pUkOrHggFmzfmtttN56uffrRwuEK/b5uxYNCrc/zBIOiujHUFh/j5X4SfhSFLNmJLArOLopj7MIKnq/k+VR+ttGomUPzBaeec3uGayeFH3AXswSTS67+11D5elM4umD1G+GEX6yxfNilSOcNsxYSHaPXu6hpI5kHxIKqczZU7TioitSwWr1N2ILQ7y4hsLldZt1bQ61zs7NOOclRN4S0ravN4AFPjsi//2WjmwaVyYy6G kxqe7IB6 sDoYr8OUFcnp5LK9GISV9XsGGJqYnbspEL6M3hrI5+K73fCX2h8HzwrfJlAIrjqF9sObCGY+j7UnfeiKBpyYo6pctxoXofdsONKq4rpG+jqsMaafxMI2UupSMIAsS5xW0UeW4BsOarreIgZepAaU2NaFy19UTDYIUQlDdGTcUQmAllXNxCik+dZrm3oweCB2pPYhv8DyeA3lCAuHg92H3kmPG2G4zG5DUPgh3+7KBUkaUBFpvFrPNdsGQqK8/sGFBlWPH 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 Wed, Jun 28, 2023 at 12:18=E2=80=AFAM Suren Baghdasaryan wrote: > > Enable handle_userfault to operate under VMA lock by releasing VMA lock > instead of mmap_lock and retrying. Note that FAULT_FLAG_RETRY_NOWAIT > should never be used when handling faults under per-VMA lock protection > because that would break the assumption that lock is dropped on retry. > > Signed-off-by: Suren Baghdasaryan > --- > fs/userfaultfd.c | 39 ++++++++++++++++++--------------------- > include/linux/mm.h | 39 +++++++++++++++++++++++++++++++++++++++ > mm/filemap.c | 8 -------- > mm/memory.c | 9 --------- > 4 files changed, 57 insertions(+), 38 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 4e800bb7d2ab..d019e7df6f15 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -277,17 +277,16 @@ static inline struct uffd_msg userfault_msg(unsigne= d long address, > * hugepmd ranges. > */ > static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ct= x, > - struct vm_area_struct *vma, > - unsigned long address, > - unsigned long flags, > - unsigned long reason) > + struct vm_fault *vmf, > + unsigned long reason) > { > + struct vm_area_struct *vma =3D vmf->vma; > pte_t *ptep, pte; > bool ret =3D true; > > - mmap_assert_locked(ctx->mm); > + assert_fault_locked(ctx->mm, vmf); > > - ptep =3D hugetlb_walk(vma, address, vma_mmu_pagesize(vma)); > + ptep =3D hugetlb_walk(vma, vmf->address, vma_mmu_pagesize(vma)); > if (!ptep) > goto out; > > @@ -308,10 +307,8 @@ static inline bool userfaultfd_huge_must_wait(struct= userfaultfd_ctx *ctx, > } > #else > static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ct= x, > - struct vm_area_struct *vma, > - unsigned long address, > - unsigned long flags, > - unsigned long reason) > + struct vm_fault *vmf, > + unsigned long reason) > { > return false; /* should never get here */ > } > @@ -325,11 +322,11 @@ static inline bool userfaultfd_huge_must_wait(struc= t userfaultfd_ctx *ctx, > * threads. > */ > static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx, > - unsigned long address, > - unsigned long flags, > + struct vm_fault *vmf, > unsigned long reason) > { > struct mm_struct *mm =3D ctx->mm; > + unsigned long address =3D vmf->address; > pgd_t *pgd; > p4d_t *p4d; > pud_t *pud; > @@ -337,7 +334,7 @@ static inline bool userfaultfd_must_wait(struct userf= aultfd_ctx *ctx, > pte_t *pte; > bool ret =3D true; > > - mmap_assert_locked(mm); > + assert_fault_locked(mm, vmf); > > pgd =3D pgd_offset(mm, address); > if (!pgd_present(*pgd)) > @@ -445,7 +442,7 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, uns= igned long reason) > * Coredumping runs without mmap_lock so we can only check that > * the mmap_lock is held, if PF_DUMPCORE was not set. > */ > - mmap_assert_locked(mm); > + assert_fault_locked(mm, vmf); > > ctx =3D vma->vm_userfaultfd_ctx.ctx; > if (!ctx) > @@ -522,8 +519,11 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, un= signed long reason) > * and wait. > */ > ret =3D VM_FAULT_RETRY; > - if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT) > + if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT) { > + /* Per-VMA lock is expected to be dropped on VM_FAULT_RET= RY */ > + BUG_ON(vmf->flags & FAULT_FLAG_RETRY_NOWAIT); Sorry, this should have been: + BUG_ON(vmf->flags & FAULT_FLAG_VMA_LOCK); > goto out; > + } > > /* take the reference before dropping the mmap_lock */ > userfaultfd_ctx_get(ctx); > @@ -561,15 +561,12 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, u= nsigned long reason) > spin_unlock_irq(&ctx->fault_pending_wqh.lock); > > if (!is_vm_hugetlb_page(vma)) > - must_wait =3D userfaultfd_must_wait(ctx, vmf->address, vm= f->flags, > - reason); > + must_wait =3D userfaultfd_must_wait(ctx, vmf, reason); > else > - must_wait =3D userfaultfd_huge_must_wait(ctx, vma, > - vmf->address, > - vmf->flags, reason= ); > + must_wait =3D userfaultfd_huge_must_wait(ctx, vmf, reason= ); > if (is_vm_hugetlb_page(vma)) > hugetlb_vma_unlock_read(vma); > - mmap_read_unlock(mm); > + release_fault_lock(vmf); > > if (likely(must_wait && !READ_ONCE(ctx->released))) { > wake_up_poll(&ctx->fd_wqh, EPOLLIN); > diff --git a/include/linux/mm.h b/include/linux/mm.h > index fec149585985..70bb2f923e33 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -705,6 +705,17 @@ static inline bool vma_try_start_write(struct vm_are= a_struct *vma) > return true; > } > > +static inline void vma_assert_locked(struct vm_area_struct *vma) > +{ > + int mm_lock_seq; > + > + if (__is_vma_write_locked(vma, &mm_lock_seq)) > + return; > + > + lockdep_assert_held(&vma->vm_lock->lock); > + VM_BUG_ON_VMA(!rwsem_is_locked(&vma->vm_lock->lock), vma); > +} > + > static inline void vma_assert_write_locked(struct vm_area_struct *vma) > { > int mm_lock_seq; > @@ -723,6 +734,23 @@ static inline void vma_mark_detached(struct vm_area_= struct *vma, bool detached) > struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm, > unsigned long address); > > +static inline > +void assert_fault_locked(struct mm_struct *mm, struct vm_fault *vmf) > +{ > + if (vmf->flags & FAULT_FLAG_VMA_LOCK) > + vma_assert_locked(vmf->vma); > + else > + mmap_assert_locked(mm); > +} > + > +static inline void release_fault_lock(struct vm_fault *vmf) > +{ > + if (vmf->flags & FAULT_FLAG_VMA_LOCK) > + vma_end_read(vmf->vma); > + else > + mmap_read_unlock(vmf->vma->vm_mm); > +} > + > #else /* CONFIG_PER_VMA_LOCK */ > > static inline void vma_init_lock(struct vm_area_struct *vma) {} > @@ -736,6 +764,17 @@ static inline void vma_assert_write_locked(struct vm= _area_struct *vma) {} > static inline void vma_mark_detached(struct vm_area_struct *vma, > bool detached) {} > > +static inline > +void assert_fault_locked(struct mm_struct *mm, struct vm_fault *vmf) > +{ > + mmap_assert_locked(mm); > +} > + > +static inline void release_fault_lock(struct vm_fault *vmf) > +{ > + mmap_read_unlock(vmf->vma->vm_mm); > +} > + > #endif /* CONFIG_PER_VMA_LOCK */ > > /* > diff --git a/mm/filemap.c b/mm/filemap.c > index 7ee078e1a0d2..d4d8f474e0c5 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1699,14 +1699,6 @@ static int __folio_lock_async(struct folio *folio,= struct wait_page_queue *wait) > return ret; > } > > -static void release_fault_lock(struct vm_fault *vmf) > -{ > - if (vmf->flags & FAULT_FLAG_VMA_LOCK) > - vma_end_read(vmf->vma); > - else > - mmap_read_unlock(vmf->vma->vm_mm); > -} > - > /* > * Return values: > * 0 - folio is locked. > diff --git a/mm/memory.c b/mm/memory.c > index 76c7907e7286..c6c759922f39 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -5294,15 +5294,6 @@ struct vm_area_struct *lock_vma_under_rcu(struct m= m_struct *mm, > if (!vma_start_read(vma)) > goto inval; > > - /* > - * Due to the possibility of userfault handler dropping mmap_lock= , avoid > - * it for now and fall back to page fault handling under mmap_loc= k. > - */ > - if (userfaultfd_armed(vma)) { > - vma_end_read(vma); > - goto inval; > - } > - > /* Check since vm_start/vm_end might change before we lock the VM= A */ > if (unlikely(address < vma->vm_start || address >=3D vma->vm_end)= ) { > vma_end_read(vma); > -- > 2.41.0.162.gfafddb0af9-goog >