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 9FB45E80A8A for ; Thu, 28 Sep 2023 00:47:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B00886B012C; Wed, 27 Sep 2023 20:46:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AB0C46B012D; Wed, 27 Sep 2023 20:46:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99FC66B0132; Wed, 27 Sep 2023 20:46:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 852A86B012C for ; Wed, 27 Sep 2023 20:46:59 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 487B51CA26D for ; Thu, 28 Sep 2023 00:46:59 +0000 (UTC) X-FDA: 81284166558.05.8B57E18 Received: from mail-yw1-f176.google.com (mail-yw1-f176.google.com [209.85.128.176]) by imf09.hostedemail.com (Postfix) with ESMTP id 81F1414001C for ; Thu, 28 Sep 2023 00:46:57 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="DB1/ABnQ"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of surenb@google.com designates 209.85.128.176 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=1695862017; 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=h/pfgqunJ1tyJvfdVMgr/pBK79yu4RSXWDU+4BRfJcQ=; b=Sfl8w1RHP4VrCf0QclJdtYxqQ14l/2J+U/ao5ASfEWfM6Brf+AH4PcE8mypp2HftuDZ1oC NxA/L3nVrgmvsQgGoMm/vGaGbgdHN9BSRLjQmRGFoMIcdcrTGbirMtXXGKEAfERGjw8SzW rGyOfc+iBW3Zxi8CnRs4ZDNlF68Ul5A= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b="DB1/ABnQ"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of surenb@google.com designates 209.85.128.176 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695862017; a=rsa-sha256; cv=none; b=wm3e0oFhrfOqCb3ueBP7fNMven/ThlQ69XsKRTmCYJ07tqMkl0hpu3POvbmP46rXuDc0Gy 2Htcudq1SYSdStww6OalCzJ4Or/GzTJsgtBa3wDQ65HrN4fQHU4tEssYgJ9XBjZJXlryRT 9QeWfw4dySL1MeCdTGg6nyNXtGXGLNQ= Received: by mail-yw1-f176.google.com with SMTP id 00721157ae682-59c268676a9so152419227b3.0 for ; Wed, 27 Sep 2023 17:46:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1695862016; x=1696466816; 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=h/pfgqunJ1tyJvfdVMgr/pBK79yu4RSXWDU+4BRfJcQ=; b=DB1/ABnQRqbuPfuZjxhsmdoX+44r5ljeo7fxm6mqGIOwcoEPr9BG2OMcUXxPVc0DCx 0N8BTPWwiCIYM5mbMIp+dY4fZ3amZiy4d+sN95WY96PnFX30qTpVOfGDYMnOLWT5jwiC /TsM7hW1iWcH+G9g12W9Ih/NHciRrFBPXFgiTSb894QfmzMcr/92nzbLBx2Dh74y3T/C lNvXodlBOrbMj4U6SDXCzfHtau1EAVWGjShUlmj/r3JEvMvZ1QkZ6dZEasH7mR6VWcLn 3hdElj5HicfCbbbO02GwKsyHr3uJqaFAJoPOBNKHJyVIsp+g+9+u3xpEO8y3JhSOWgcs nIFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695862016; x=1696466816; 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=h/pfgqunJ1tyJvfdVMgr/pBK79yu4RSXWDU+4BRfJcQ=; b=j80/ov4OK0ZMr9GQ5ymoUnc6fumFWMOOPgInkU8/HmWQ8DfL1c4YTNn7FNcnPjotSC 9GG66nA4NxjMHslAcSfMj7OelUUPycrm8MtngkBckgTaEr2VAKA8u++4tA6qyF4jQXQl GwYZntlI+3CRwGCO4yIowGUuroVJbx4Oepq8nCtyZGR+mFWiXT/dQQfXvkfH47Eka5Ig RmBxpGmuFQWv8F8BYIOA+S/WbSS8lm0EDqZqjYy0AyuJhT69q1X+3gz5cvvIpnaWrQgI vqFw0D8HkeS5bvjsSmtfvv5TYLQBjRJy2+zXyboqXbZhB+YhpgpjmpkKhsgZiiOJ+GfJ WitA== X-Gm-Message-State: AOJu0YwY23jNmh6wqghPF4uU3uld0XxhHAeLPQgetmO0JMRjK9QgKOum BGkGdSHDvz3NTrsStlg+HfYvc0C2oc71cZPf4ycr0w== X-Google-Smtp-Source: AGHT+IF8p/Oukzj4eESuMaPWa8M4Q6IjKjoHi60ZXMQyU1U6S7e6sEQ6AWKlW3MaxUMOe08wUuOjPWWHWH7tT82EOvg= X-Received: by 2002:a81:4fc6:0:b0:5a1:ed8d:111f with SMTP id d189-20020a814fc6000000b005a1ed8d111fmr3639613ywb.1.1695862016273; Wed, 27 Sep 2023 17:46:56 -0700 (PDT) MIME-Version: 1.0 References: <20230927052505.2855872-1-willy@infradead.org> <20230927052505.2855872-4-willy@infradead.org> In-Reply-To: <20230927052505.2855872-4-willy@infradead.org> From: Suren Baghdasaryan Date: Wed, 27 Sep 2023 17:46:42 -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: 81F1414001C X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: shjfhf7fno94qcjao49i4g1zgdetj1ph X-HE-Tag: 1695862017-534675 X-HE-Meta: U2FsdGVkX1/50kN4wclO80PwhKxANQaLO/CY8HKQJhm2l4qkAZAuiYoV9zRaeWi6YrNdeHYTULEJa5oyuIhJSwtR6B1zg/+ZsWUG+gOqME+LR3qDa/uXEMGBYEi/E+qb+OBDSLhUfE3L58tVJ09aaV2gYviBHNAeeEyNTFroFTGnZRvGAcKSsMmpgg1X4rrb+OMfBvE2Zpaq3xXh7j+g+PCQqrX3kcYt6FGnGRmK18b+aloW+Mh+WpCYtY+kA2jvQmCrnVDwGQ4+diFuGD3aWX7KPP+aB/twcVjjdB9uhkCXwhgCOBhBxOENWTNt+mFSL1rIkODphxqJEFVweJoWgCNiODhksF9K2bbyNM8w5YRde0qsp3sEWiVy84Arltpwkcv6Nm5yMu4C3omvDxFX+ueWXw2B1KbxYUqbSRra7GNd0F4PK74UxeZJQ1eVWAtqZnlJyLCZIgmcABAzhZYnat88ina/PLB4WzX294pnezs48eENaLjgEqE+YxrIrK+CzDV2vgd6yP3P9DWLK2vfC8HcNQCTBp1o1SbPFfF9rNJqk8rmLDL7LkrpcNbuj644dUznNfhkFhhXmfw4+Kuo1K5Q1E2iDQtS9koOFQ9PzBAUMGXKEGXZNP/JF7NRZPhkU9rKrJDBrw49SiAZ4IQDtshbzYAowJWpvFiboU+6jp9GPrr1CFGFgHxKUYOo1Z7rZGL3g5vRq511C28nA+J/hJ2SVI1uEYmuBPyd9OrO7mpP+tJNRgw6RbzACWo9qnId+zwX4uol54p9BJIuZ5TG7DiEkaP3Ny7sTummCeMREYNTEv7cK8oCB2vKRz/2l8NhOOaYEQCzoXo9NYfdWbCdWs7CX4fSn37H0sl2QjXSOVIV7o9lmFOL0nI0rH7+HdDKk4wFwF8XFCxerIZ+ijcTCTAYhp7xxSAdcDMU1UMm7re3ySJMueFOmN1Wu5omMFpda0pNg5/XTwDB26Pdw1R Jr9R0bAZ HlotsyjJTAkZvsrHPVSHsAxsjfsIO4TfRBUIG5VMJmEqdwCS6HDBpoPEd7NQCNwpsY64uvcsRQdtHODDUinX9RPnqhpRMv8m46V3766RUwpVm8VoJj0C7zzIRDsvcwbJzHegmC3izzRotD6b438ITFOx7tmCYU/fGycDoVjKjoeR8madH+ZUcl1k3IW5LK2X9lnTB9uwpCogYy6iTzDWn/CYFFvNmMTaKVbVBnmpB4o2XLrVMnA3mdYtOUumtisgA9T9NSUBHRBa6M/MoxdKd3LjxwigZ7P0AAO3L 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, 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 *vmf= ) > +{ > + struct vm_area_struct *vma =3D vmf->vma; > + > + if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK)= ) > + 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_fault = *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(v= ma) { vma_end_read(vma); return VM_FAULT_RETRY; } > + if (ret) > + return ret; > > ret =3D __do_fault(vmf); > if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_R= ETRY))) > -- > 2.40.1 >