Message ID | 20230627042321.1763765-6-surenb@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp7941261vqr; Mon, 26 Jun 2023 21:36:45 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4xuLew/9CkCi8h8y/f+jp46DKJ3V70nr0ogia2xn1vqIK8JEHYid57Tj9RGS3s1x7dTj0J X-Received: by 2002:a2e:8304:0:b0:2b1:c1ae:73e3 with SMTP id a4-20020a2e8304000000b002b1c1ae73e3mr17883500ljh.15.1687840604954; Mon, 26 Jun 2023 21:36:44 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687840604; cv=none; d=google.com; s=arc-20160816; b=A3IySgiqDCNM6XDh9TN4M8pk1q/TN/+af90EBGSsmjDj1Cv4yKZQn+Z+Cq0mUO7iTq KlmhopMKfKeUbhenKiZxNQjOWMSby+yzNybHiNIti96pkzS7NsmwXmIOAW0oJq6ulslU sqnB/2fADe3kIyC5VgaEGuiEWr5GCTItN3OMRYTUN1I8z+qSCGbQK45tWGRYZmPgNNRc GMWwrL6KvGlNAMa8yFGANIbiN1A1wq8Va2Y+69b4kKHPYD3oua4TmlgGXkvZOlcwcAj7 naXbMIA6i6dJHi4q7bjFF9MW0CGZthXcS9DO3XQsrGOfN4wrU7c2BKOOnJPLfbL6b8Pv 1gLw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=/bqEyfDb2JHBciJ5X1JfWPHzjf7SmvaClBSwV4xMi6U=; fh=RMSYO7NH5mgRq+Q9Sd8uvtKjjFjspH3ZqZT6M3GUwas=; b=owquepaO9B37nBSDR8yDiuf6Jijcw7AjUum73+Vs5zr10WSR/+sZouKhcdB9RmzIcn bhE32g2X9xo68Equo+JLzetoq3dMgLFvhrdAhefqCo4nUwQ362uleM+UY16nKmnb6T/X 4/4nBZU1eQ+WiaX5MJBwvS3WaQMZqTUsNX+f+IJZw6vL7EvDBfZxHhMxG8ylSab9vxY0 tPZo8ngso1P8z9q5vNnD+HuEI8VEhbIyzlmY1qt1YuMBtZi0rJ3waVyKcuXn63gky6jz QYpa8oFqUqvzXtxDC/thEmxRMyQxbisEuSAUDzh2MAnYV/H62xXDUx+2OSoH2drw8isr /0Wg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=OvvulEsW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l7-20020aa7c3c7000000b0051b1dd0a1acsi3586359edr.277.2023.06.26.21.36.20; Mon, 26 Jun 2023 21:36:44 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=OvvulEsW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230385AbjF0EYI (ORCPT <rfc822;nicolai.engesland@gmail.com> + 99 others); Tue, 27 Jun 2023 00:24:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56352 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230179AbjF0EXl (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 27 Jun 2023 00:23:41 -0400 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8B81A1734 for <linux-kernel@vger.kernel.org>; Mon, 26 Jun 2023 21:23:36 -0700 (PDT) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-56cf9a86277so52586367b3.3 for <linux-kernel@vger.kernel.org>; Mon, 26 Jun 2023 21:23:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687839816; x=1690431816; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=/bqEyfDb2JHBciJ5X1JfWPHzjf7SmvaClBSwV4xMi6U=; b=OvvulEsWmawhmtqOLDyG7NX+8MMLCKyrPheIcVIJBGuloUk350HzepAZTTg9GSNFTq Zxyei2vg/f2B5jkrZYoGKPbRxvuLjH35sLV4l1wOBTJafVD0HTbDUzbP6jPQfOdU+KaG 3gIkRMxtJ9DgS5SW0Kt3bWfeu+sXaplrrhq1B7dNLrmsE2meL4KQdekBOG7sWz0zAe0N lkYsG+/en/JT0DT8eW/ip3lW+AyM8tArkN2trbjAuP+aG5gijB434ikFW3ucpom8cFWS 1HC6TfIWomgbHMNnYthALNU+Ilov/TeO7Y284bIljOOAHiqgvbBCXiM9wMHeW6Wjn6nD V8ng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687839816; x=1690431816; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/bqEyfDb2JHBciJ5X1JfWPHzjf7SmvaClBSwV4xMi6U=; b=TmFz38ifxnThILzvWHCIlVd23hbL4J1iLO8aKNLZfOOAXCQ7IOUB3Lm9m1FEp/dRUO JQgGYLuxOSpLBl68MXw+yH2Gxbqw06M7NLg21CX3yKtlqX+T7gcK+kWUAHjRYYg6kwYM n58FVJJ9ZfmDAZa6r7gRUTj96WesgeWiPtk6o+xJBABX8t8vfdJhnAoJC68YWqTnh2iw QJpN8n/aEKCYmx7wZRW6ogw+zF+sGLb9hMa1f3nLAWX8z8YAFVidnTciKcBxwiie6mZg jFoQNYYyYHJmwIkLFVAHbJ9BmsDX09RZtDjOWcm9deiHKWVZelLid4CFn4iCGrUr43DE pChw== X-Gm-Message-State: AC+VfDz/TqcoYEy8G10nlPDlE1FPNoMsjrHlePJhr0wuyiLaFY85MnWW 7KoUq22FfbiD8kQFwSNAwZE4jX+t2ps= X-Received: from surenb-desktop.mtv.corp.google.com ([2620:15c:211:201:5075:f38d:ce2f:eb1b]) (user=surenb job=sendgmr) by 2002:a25:d3c8:0:b0:bac:adb8:a605 with SMTP id e191-20020a25d3c8000000b00bacadb8a605mr6458407ybf.2.1687839815785; Mon, 26 Jun 2023 21:23:35 -0700 (PDT) Date: Mon, 26 Jun 2023 21:23:18 -0700 In-Reply-To: <20230627042321.1763765-1-surenb@google.com> Mime-Version: 1.0 References: <20230627042321.1763765-1-surenb@google.com> X-Mailer: git-send-email 2.41.0.162.gfafddb0af9-goog Message-ID: <20230627042321.1763765-6-surenb@google.com> Subject: [PATCH v3 5/8] mm: make folio_lock_fault indicate the state of mmap_lock upon return From: Suren Baghdasaryan <surenb@google.com> 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, surenb@google.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" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769829149973011900?= X-GMAIL-MSGID: =?utf-8?q?1769829149973011900?= |
Series |
Per-VMA lock support for swap and userfaults
|
|
Commit Message
Suren Baghdasaryan
June 27, 2023, 4:23 a.m. UTC
folio_lock_fault might drop mmap_lock before returning and to extend it
to work with per-VMA locks, the callers will need to know whether the
lock was dropped or is still held. Introduce new fault_flag to indicate
whether the lock got dropped and store it inside vm_fault flags.
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
include/linux/mm_types.h | 1 +
mm/filemap.c | 2 ++
2 files changed, 3 insertions(+)
Comments
Suren Baghdasaryan <surenb@google.com> writes: > folio_lock_fault might drop mmap_lock before returning and to extend it > to work with per-VMA locks, the callers will need to know whether the > lock was dropped or is still held. Introduce new fault_flag to indicate > whether the lock got dropped and store it inside vm_fault flags. > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > --- > include/linux/mm_types.h | 1 + > mm/filemap.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 79765e3dd8f3..6f0dbef7aa1f 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -1169,6 +1169,7 @@ enum fault_flag { > FAULT_FLAG_UNSHARE = 1 << 10, > FAULT_FLAG_ORIG_PTE_VALID = 1 << 11, > FAULT_FLAG_VMA_LOCK = 1 << 12, > + FAULT_FLAG_LOCK_DROPPED = 1 << 13, Minor nit but this should also be added to the enum documentation comment above this. > }; > > typedef unsigned int __bitwise zap_flags_t; > diff --git a/mm/filemap.c b/mm/filemap.c > index 87b335a93530..8ad06d69895b 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1723,6 +1723,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > return VM_FAULT_RETRY; > > mmap_read_unlock(mm); > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > if (vmf->flags & FAULT_FLAG_KILLABLE) > folio_wait_locked_killable(folio); > else > @@ -1735,6 +1736,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > ret = __folio_lock_killable(folio); > if (ret) { > mmap_read_unlock(mm); > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > return VM_FAULT_RETRY; > } > } else {
On Mon, Jun 26, 2023 at 09:23:18PM -0700, Suren Baghdasaryan wrote: > folio_lock_fault might drop mmap_lock before returning and to extend it > to work with per-VMA locks, the callers will need to know whether the > lock was dropped or is still held. Introduce new fault_flag to indicate > whether the lock got dropped and store it inside vm_fault flags. > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > --- > include/linux/mm_types.h | 1 + > mm/filemap.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 79765e3dd8f3..6f0dbef7aa1f 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -1169,6 +1169,7 @@ enum fault_flag { > FAULT_FLAG_UNSHARE = 1 << 10, > FAULT_FLAG_ORIG_PTE_VALID = 1 << 11, > FAULT_FLAG_VMA_LOCK = 1 << 12, > + FAULT_FLAG_LOCK_DROPPED = 1 << 13, > }; > > typedef unsigned int __bitwise zap_flags_t; > diff --git a/mm/filemap.c b/mm/filemap.c > index 87b335a93530..8ad06d69895b 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1723,6 +1723,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > return VM_FAULT_RETRY; > > mmap_read_unlock(mm); > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > if (vmf->flags & FAULT_FLAG_KILLABLE) > folio_wait_locked_killable(folio); > else > @@ -1735,6 +1736,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > ret = __folio_lock_killable(folio); > if (ret) { > mmap_read_unlock(mm); > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > return VM_FAULT_RETRY; > } > } else { IIRC we've discussed about this bits in previous version, and the consensus was that we don't need yet another flag? Just to recap: I think relying on RETRY|COMPLETE would be enough for vma lock, as NOWAIT is only used by gup while not affecting vma lockings, no? As mentioned in the other reply, even COMPLETE won't appear for vma lock path yet afaict, so mostly only RETRY matters here and it can 100% imply a lock release happened. It's just that it's very easy to still cover COMPLETE altogether in this case, being prepared for any possible shared support on vma locks, IMHO. Thanks,
On Tue, Jun 27, 2023 at 8:32 AM Peter Xu <peterx@redhat.com> wrote: > > On Mon, Jun 26, 2023 at 09:23:18PM -0700, Suren Baghdasaryan wrote: > > folio_lock_fault might drop mmap_lock before returning and to extend it > > to work with per-VMA locks, the callers will need to know whether the > > lock was dropped or is still held. Introduce new fault_flag to indicate > > whether the lock got dropped and store it inside vm_fault flags. > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > --- > > include/linux/mm_types.h | 1 + > > mm/filemap.c | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > > index 79765e3dd8f3..6f0dbef7aa1f 100644 > > --- a/include/linux/mm_types.h > > +++ b/include/linux/mm_types.h > > @@ -1169,6 +1169,7 @@ enum fault_flag { > > FAULT_FLAG_UNSHARE = 1 << 10, > > FAULT_FLAG_ORIG_PTE_VALID = 1 << 11, > > FAULT_FLAG_VMA_LOCK = 1 << 12, > > + FAULT_FLAG_LOCK_DROPPED = 1 << 13, > > }; > > > > typedef unsigned int __bitwise zap_flags_t; > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 87b335a93530..8ad06d69895b 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -1723,6 +1723,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > return VM_FAULT_RETRY; > > > > mmap_read_unlock(mm); > > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > > if (vmf->flags & FAULT_FLAG_KILLABLE) > > folio_wait_locked_killable(folio); > > else > > @@ -1735,6 +1736,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > ret = __folio_lock_killable(folio); > > if (ret) { > > mmap_read_unlock(mm); > > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > > return VM_FAULT_RETRY; > > } > > } else { > > IIRC we've discussed about this bits in previous version, and the consensus > was that we don't need yet another flag? Just to recap: I think relying on > RETRY|COMPLETE would be enough for vma lock, as NOWAIT is only used by gup > while not affecting vma lockings, no? Sorry for missing that point. I focused on making VMA locks being dropped for RETRY|COMPLETE and forgot to check after that change if RETRY|COMPLETE is enough indication to conclude that VMA lock is dropped. Looking at that now, I'm not sure that would be always true for file-backed page faults (including shmem_fault()), but we do not handle them under VMA locks for now anyway, so this indeed seems like a safe assumption. When Matthew implements file-backed support he needs to be careful to ensure this rule still holds. With your suggestions to drop the VMA lock at the place where we return RETRY this seems to indeed eliminate the need for FAULT_FLAG_LOCK_DROPPED and simplifies things. I'll try that approach and see if anything blows up. > > As mentioned in the other reply, even COMPLETE won't appear for vma lock > path yet afaict, so mostly only RETRY matters here and it can 100% imply a > lock release happened. It's just that it's very easy to still cover > COMPLETE altogether in this case, being prepared for any possible shared > support on vma locks, IMHO. Yes and I do introduce one place where we use COMPLETE with VMA locks, so will cover it the same way as for RETRY. Thanks, Suren. > > Thanks, > > -- > Peter Xu >
On Tue, Jun 27, 2023 at 1:09 AM Alistair Popple <apopple@nvidia.com> wrote: > > > Suren Baghdasaryan <surenb@google.com> writes: > > > folio_lock_fault might drop mmap_lock before returning and to extend it > > to work with per-VMA locks, the callers will need to know whether the > > lock was dropped or is still held. Introduce new fault_flag to indicate > > whether the lock got dropped and store it inside vm_fault flags. > > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > --- > > include/linux/mm_types.h | 1 + > > mm/filemap.c | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > > index 79765e3dd8f3..6f0dbef7aa1f 100644 > > --- a/include/linux/mm_types.h > > +++ b/include/linux/mm_types.h > > @@ -1169,6 +1169,7 @@ enum fault_flag { > > FAULT_FLAG_UNSHARE = 1 << 10, > > FAULT_FLAG_ORIG_PTE_VALID = 1 << 11, > > FAULT_FLAG_VMA_LOCK = 1 << 12, > > + FAULT_FLAG_LOCK_DROPPED = 1 << 13, > > Minor nit but this should also be added to the enum documentation > comment above this. Thanks! Sounds like we will be dripping the new flag, so hopefully I won't need to document it :) > > > }; > > > > typedef unsigned int __bitwise zap_flags_t; > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 87b335a93530..8ad06d69895b 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -1723,6 +1723,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > return VM_FAULT_RETRY; > > > > mmap_read_unlock(mm); > > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > > if (vmf->flags & FAULT_FLAG_KILLABLE) > > folio_wait_locked_killable(folio); > > else > > @@ -1735,6 +1736,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > ret = __folio_lock_killable(folio); > > if (ret) { > > mmap_read_unlock(mm); > > + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > > return VM_FAULT_RETRY; > > } > > } else { >
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 79765e3dd8f3..6f0dbef7aa1f 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -1169,6 +1169,7 @@ enum fault_flag { FAULT_FLAG_UNSHARE = 1 << 10, FAULT_FLAG_ORIG_PTE_VALID = 1 << 11, FAULT_FLAG_VMA_LOCK = 1 << 12, + FAULT_FLAG_LOCK_DROPPED = 1 << 13, }; typedef unsigned int __bitwise zap_flags_t; diff --git a/mm/filemap.c b/mm/filemap.c index 87b335a93530..8ad06d69895b 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1723,6 +1723,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) return VM_FAULT_RETRY; mmap_read_unlock(mm); + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; if (vmf->flags & FAULT_FLAG_KILLABLE) folio_wait_locked_killable(folio); else @@ -1735,6 +1736,7 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) ret = __folio_lock_killable(folio); if (ret) { mmap_read_unlock(mm); + vmf->flags |= FAULT_FLAG_LOCK_DROPPED; return VM_FAULT_RETRY; } } else {