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 3F2CBE82CBC for ; Thu, 28 Sep 2023 01:03:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 84D178D00A3; Wed, 27 Sep 2023 21:03:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7FD268D0002; Wed, 27 Sep 2023 21:03:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6EC348D00A3; Wed, 27 Sep 2023 21:03:05 -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 5F6908D0002 for ; Wed, 27 Sep 2023 21:03:05 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 33C3AB3E6A for ; Thu, 28 Sep 2023 01:03:05 +0000 (UTC) X-FDA: 81284207130.17.9102189 Received: from mail-yw1-f178.google.com (mail-yw1-f178.google.com [209.85.128.178]) by imf12.hostedemail.com (Postfix) with ESMTP id 767E740014 for ; Thu, 28 Sep 2023 01:03:03 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=lquVJt1t; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf12.hostedemail.com: domain of surenb@google.com designates 209.85.128.178 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695862983; 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=IAAvGRxlFRjHLREDagbRnjoe2SlQCMxxZvqz4VWpLwM=; b=1NE/dvFB3LoV/bknrkGHfYTHPbtYqhd6jW3qZ3BirMz9lXenO2dvf9f3j6JpzJzHqn2fr2 an4ooDApVtE2jNWlEHnK38C35jccTG8r2nSmEPCA4aPNBHScKCEV7f7W/oLbvwkfhikP9m 4J5ASvTuPY932q+oddnpShs8YWA7ias= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=lquVJt1t; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf12.hostedemail.com: domain of surenb@google.com designates 209.85.128.178 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695862983; a=rsa-sha256; cv=none; b=mOlrTOxTn04R+AkFrueSHhHOXkHmYeokjBKGfAJpwk8LQ+Wtf94U/7ggmCiAhknC/8Aya0 PiP64e+zlHSWGfQGZEy7Bp3PezL+2nwBAnPyT3KNrbgiDzg5W60Lj5R8NcY/L0uMtNz3q/ UFjadi88O9LBWkSQU/0luSCiHIWIjNU= Received: by mail-yw1-f178.google.com with SMTP id 00721157ae682-5a1ec43870cso53000997b3.0 for ; Wed, 27 Sep 2023 18:03:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1695862982; x=1696467782; darn=kvack.org; 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=IAAvGRxlFRjHLREDagbRnjoe2SlQCMxxZvqz4VWpLwM=; b=lquVJt1tHRjYpUHPeiY/FSXcfoJevf9HrU9Smi5UxgW/Q+qMQWxqQzCMCL5OhQurGg Ix/tSPSkYucAzUfmT1nwDwo5Ib5DsCxQqOgr4myjA/7diX7tBPs62aSIxyxyCrHOKW0i O8IlTdbuKmVYPrEbaewiMvmgof7OSPfzjvAuAiaNiqAt5TWN3IJlbAHLN1q59t4pXZP7 ruF5SnziwZvW0ZB1PGtyXpGjEU4cn6ygm1kxNXfF+cqIz5TmNWtR0xKT0EL5IbfFQSSc Fv7puwiKnqC/bML2co0dcg0ASuD26BpyVYBWfmVnU41Lvl1nQgp48OmW7UdRq5YP3fT3 29/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695862982; x=1696467782; 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=IAAvGRxlFRjHLREDagbRnjoe2SlQCMxxZvqz4VWpLwM=; b=ZNGjVI3JLy/sRRLZ4MoGg2UAl88pSv6hO2z39hmUsSc2miI4M737NFQBnEXJKwqqQq 0WY8C/ZKkoUMFMO+AIuYP2d+DL591b4dw5N5VhQELbzXsyMHjAuMsHpZjSoXnZmnoSnd Fhk4hWgQU3Nqo7djjYiO1k19+zBxJTpZmLml5F04J6NcgEbEgGepGN6eubhy2ANvq5+h nhtyozUAGiBxGbutu2+N9RalnlYN/qP89a5paGVc5E+r0pur71MRdr1WyU0kLhPmgPVN Ie3mXwgAlVtg/7r0WLUn3zv54VQKzI6WyM6Lx8ggNiCqY7s4IAKMuOMZ1qVU9qYiwBnA Ovhw== X-Gm-Message-State: AOJu0YzchNqmD8svEP0nCf3tJpgf/R6oWZZlFs7kq3iNcSW0hFTFNV3v ujYl0LXHaeW8fq+CcRNKNs3Tz9vsTvW5uSlUj6I6iw== X-Google-Smtp-Source: AGHT+IGeFcoYDJ4r6SPALoPny+ODcUXhmYvtIpYJ2e5d+Tvk/w1EwHOfOazFiPifiMzt+kZ5teXwi04iC3gbCO1l8XQ= X-Received: by 2002:a0d:db4b:0:b0:583:f80d:f461 with SMTP id d72-20020a0ddb4b000000b00583f80df461mr3801692ywe.8.1695862982311; Wed, 27 Sep 2023 18:03:02 -0700 (PDT) MIME-Version: 1.0 References: <20230927052505.2855872-1-willy@infradead.org> <20230927052505.2855872-4-willy@infradead.org> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 27 Sep 2023 18:02:47 -0700 Message-ID: Subject: Re: [PATCH 3/6] mm: Handle shared faults under the VMA lock To: "Matthew Wilcox (Oracle)" Cc: Andrew Morton , linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 767E740014 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 9ybmok3mkjrxey9bzu74swqgj3sksmp5 X-HE-Tag: 1695862983-527003 X-HE-Meta: U2FsdGVkX1/7S2HhA8ajBLCyO+sx1jDKt0Z8cjlrPR9CEw0bdKoHQvIIvdT1bjaGekJI0DrVcI4KyG62bbQPoHUmVfmgKkNNjBL6vfS0Xc6PRDbZ0EOPeKLSqHMLX7No21P1BdGWj2wrSOriqOXbGcZjFCW451+8irknz2rKyHJFNn0ar1qIlI1yl4bMr8Gteq9Y8WKumOoNACniggLXI4Z2ZlS/TMa+4EtSYWsbRPMe6HQsqao9v1ys2O4PGgishgYrA9dLUCFS/bPdZ1RA0aocNZCD/j/e/T096WEqxX9FxuASUuyoaDCpTS4y7mMsIbygkYyen39dotjsSxQ/GX/O7pCizxFxFRXcFWeWTeWEnpRMOVwnKl3xv9CzgvVAfNiJrtbw3ytrZhw6Hdnvpuy1gO8AI9A6C2V9axU9JV+VML5LkpflGuaVyryRc23EQduJMZBycR+0QwfttM5E6ksgV76HwWX1MRR/pOXCgbQWDOw/ecXmC8ccd2YMy1YQnKHreOUhxvEKmmWs1a0LNFmQgmXlPmkqjrxeLNVw5JFYgrMMV07avFqE3o/cHiJzemvIHkehG91N5CcV1nPSHKGO3bN1pPq/SFDlLC6MOg2g55292k6l2UNqteuMcHE4cKrqbXZTQz84UkAsqYscQ4YTLbZ3LWdruY1Os3PNDu3JnkoPOU36BFMDL8+ht/fVkIfBknivZXWu9TwmMXJSVxSXKVtGyKZvUAJsFuuVItGqHTOpeMB5JsgQ/OBJH6ykwTTwPm80AmTDeQYAXmu1AEsJQ3qD5ATaX0j4VOZ8MJWdpCfpdB+ckSfVa0KwEckZDMbdDrW6gggY+wIacH+CagYmhAXRbkJhGbnWNco92bmvUiNFieuHYPXxK4morjPnQdbIeEsyNd2rh5W/SdrUQfUEnMnm64U7prhDTQ4+xtVr2WmcNe91ICSPPTlUSifIB4qtA51UOXF4kajtqQa k/ly67xS Nto10I8n/0YAkg52N2PPCVsxy4q6+4PnhDES7FFIOc4I234fIxKp8tu69H5IYd16iIQmGoB7aPzDGryYZ+wFmlEYnHqalGgjP3x5Z4/QXG285D8pBLfQMtdlW6TzQsydzpM33RQooGflW9qNlNgiSAXwQ5E5vB51RyceknbvcPOUWfY7etXI2XczkaWd48QWhMFkryxiUTk27NkfdVKSTJb4R9Bbr8yfL0LYtwEYRVGQlZ7x6bGEpazsKKxISmuFbLZ4LoJ2Phmm1APF9gFdBwn1NANIh36y1aTQtJawxMfp1BSryj8g6p3Ftjw== 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, Sep 27, 2023 at 5:46=E2=80=AFPM Suren Baghdasaryan wrote: > > On Tue, Sep 26, 2023 at 10:25=E2=80=AFPM Matthew Wilcox (Oracle) > wrote: > > > > There are many implementations of ->fault and some of them depend on > > mmap_lock being held. All vm_ops that implement ->map_pages() end > > up calling filemap_fault(), which I have audited to be sure it does > > not rely on mmap_lock. So (for now) key off ->map_pages existing > > as being a flag to indicate that it's safe to call ->fault while > > only holding the vma lock. > > > > Signed-off-by: Matthew Wilcox (Oracle) > > --- > > mm/memory.c | 22 ++++++++++++++++++---- > > 1 file changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/mm/memory.c b/mm/memory.c > > index cff78c496728..0f3da4889230 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -3042,6 +3042,21 @@ static inline void wp_page_reuse(struct vm_fault= *vmf) > > count_vm_event(PGREUSE); > > } > > > > +/* > > + * We could add a bitflag somewhere, but for now, we know that all > > + * vm_ops that have a ->map_pages have been audited and don't need > > + * the mmap_lock to be held. > > + */ > > +static inline vm_fault_t vmf_maybe_unlock_vma(const struct vm_fault *v= mf) > > +{ > > + struct vm_area_struct *vma =3D vmf->vma; > > + > > + if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOC= K)) > > + return 0; > > + vma_end_read(vma); > > + return VM_FAULT_RETRY; > > +} > > + > > static vm_fault_t vmf_anon_prepare(struct vm_fault *vmf) > > { > > struct vm_area_struct *vma =3D vmf->vma; > > @@ -4669,10 +4684,9 @@ static vm_fault_t do_shared_fault(struct vm_faul= t *vmf) > > vm_fault_t ret, tmp; > > struct folio *folio; > > > > - if (vmf->flags & FAULT_FLAG_VMA_LOCK) { > > - vma_end_read(vma); > > - return VM_FAULT_RETRY; > > - } > > + ret =3D vmf_maybe_unlock_vma(vmf); > > The name of this new function in this context does not seem > appropriate to me. The logic of this check was that we can't rely on > VMA lock since it might not be sufficient, so we have to retry with > mmap_lock instead. With this change it seems like we intentionally try > to unlock the VMA here. IMHO this would be more understandable: > > static inline bool is_vma_lock_sufficient(struct vm_area_struct *vma) { > return vma->vm_ops->map_pages !=3D NULL; > } > > static vm_fault_t vmf_anon_prepare(struct vm_fault *vmf) > { > ... > if ((vmf->flags & FAULT_FLAG_VMA_LOCK) && !is_vma_lock_sufficient= (vma) { > vma_end_read(vma); > return VM_FAULT_RETRY; > } Same comment for the rest of the patches where vmf_maybe_unlock_vma() is being used. It would be great to have this logic coded in one function like you do but I could not find an appropriate name that would convey that "we want to check if the current lock is sufficient and if not then we will drop it and retry". Maybe you or someone else can think of a good name for it? > > > + if (ret) > > + return ret; > > > > ret =3D __do_fault(vmf); > > if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT= _RETRY))) > > -- > > 2.40.1 > >