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 7C5F2EB64D9 for ; Tue, 27 Jun 2023 16:01:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 063BB8D0002; Tue, 27 Jun 2023 12:01:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 012748D0001; Tue, 27 Jun 2023 12:01:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E1C848D0002; Tue, 27 Jun 2023 12:01:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id CE8768D0001 for ; Tue, 27 Jun 2023 12:01:10 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 1467780A96 for ; Tue, 27 Jun 2023 16:01:09 +0000 (UTC) X-FDA: 80948991858.09.46F6715 Received: from mail-yw1-f172.google.com (mail-yw1-f172.google.com [209.85.128.172]) by imf17.hostedemail.com (Postfix) with ESMTP id A314D4007B for ; Tue, 27 Jun 2023 16:00:31 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=TSmneStq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf17.hostedemail.com: domain of surenb@google.com designates 209.85.128.172 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=1687881631; 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=9KkJfMIW232ztzAZaUdoyK+3frIKBC3oUzLc/0KyY7g=; b=dXqHRR38kTehSoqRKVXf5hPNcOBPPNJdrIvww4ofq0nhK9Y1zkbxcKt0ro4ouV5s3L04k3 a8N9BXP/YJ5zd4kxhYl3aL17BMYhMu5R4lgajQYLefsONH9cJMgNtY1/78wd0DNaH0HZ5p vMVuwZlePizzs3oSQ/QhGFUKKkGfz9o= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=TSmneStq; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf17.hostedemail.com: domain of surenb@google.com designates 209.85.128.172 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687881631; a=rsa-sha256; cv=none; b=GL1PHUgqOTLc74pWWaMIm3MqqSnjMC/Vx8QY6cZz+kJLATzr1/TmgMQtkRjSpK36XxIetI tHMoQ56VlkivEr5sacGxem0gd1tYNFBZ+KyY3T1z75mmslegA3t2hnFvIJH1qAt6sy5iFq 8l5r81nejK0Mifqer3zWsPn+ZnOmdj4= Received: by mail-yw1-f172.google.com with SMTP id 00721157ae682-57688a146ecso43977057b3.2 for ; Tue, 27 Jun 2023 09:00:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687881630; x=1690473630; 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=9KkJfMIW232ztzAZaUdoyK+3frIKBC3oUzLc/0KyY7g=; b=TSmneStqHeW3vCGRSk9XN9K/xAOpUNp9Vt2idzhSed8Jk+sSOVzUMYMfK5RbQI1ezV xCcyNw+2DbcLe4wWwb+sZrHHbbktJgcRITbuZtz6oydu/tUcvwox4YrknRwXpzAlmJOP orSU/BHf8zwqMob8Lmjh+Rk+lPb7uV7/TsKfBFQo78u+j2lducnvRLXzqme6BAOSma7C wm+yNe+rT3S/dvG2NXgwRWysJw6pZHxyQcFfsx8v6zz6MvXxzLTh6RSn3eg7C+0+NnyJ KA9VLZ5YOphRWjJmhl7NlekRjYrHPsGQNPRy+HRTD3mtoTXAsOZc7Mn6GtWgZTBQ0Ohc Dgog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687881630; x=1690473630; 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=9KkJfMIW232ztzAZaUdoyK+3frIKBC3oUzLc/0KyY7g=; b=XTtOqVczyN72NClqAFzH2Gih2pJnfUz9tyDCr5oqsUgpW54e3FGe/Fwy7NDIX1DoOk Zl4nO++6E18ofPpj0/6IqjuUoT5WJ4ReoIHXKDD8pZ6rhvNFNSS/cG+kowx18yL4Pg/n OxCRPKmwzIB9TdxVGQ5qZ6ugQOm9nBfl/mx6g7RqbpqEKxYbirUNRa7DkKVO7TdSB2Wp jTjydx5H9aEQmBhKN23WniS60ZABIDcYor199+9RWfejfpharZ+2JJNaqBzOl0AJq2rz OLJZnxO4vjr8f7eW/QFVot/rN0D41RbmBGEadnYQHHCDwLCkycaoI1zb/4vJmvR8qDIG TH1A== X-Gm-Message-State: AC+VfDxiOsFC8oNgwLXeUry6WBYLkvO630nSPK16cZz8J52AThI3bB6Y eg5rSbjEmccbQnJupxpvE7ias0lcpiO2xsKPO8hr1w== X-Google-Smtp-Source: ACHHUZ4+7sbj81UsRHmAbl9QlYt7ZZmVqNsMLkZDz17Ve/pdy6A0enKOXhe8I1iDZXgOZeytbthoVeE93RHK/qCLMik= X-Received: by 2002:a25:ca0b:0:b0:ba1:ce0d:a076 with SMTP id a11-20020a25ca0b000000b00ba1ce0da076mr30559075ybg.43.1687881630048; Tue, 27 Jun 2023 09:00:30 -0700 (PDT) MIME-Version: 1.0 References: <20230627042321.1763765-1-surenb@google.com> <20230627042321.1763765-6-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 27 Jun 2023 09:00:18 -0700 Message-ID: Subject: Re: [PATCH v3 5/8] mm: make folio_lock_fault indicate the state of mmap_lock upon return To: Peter Xu Cc: akpm@linux-foundation.org, 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, 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: A314D4007B X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: jjashp5uwqs3z5ciq7mx1rkrrboxrnao X-HE-Tag: 1687881631-62649 X-HE-Meta: U2FsdGVkX19gMDhS4mX3/DUEcGiYa6/MX2PhtqrFcS9uZjrtmzpXQ+bjmGLZtbsP6NSfuHIp0M62MkC7tjS4OAnHdHwfJPAkLVKleJJ1w7SrSwWaC3rv6rw4ufLl9IzG47rG055j12vwlPBoTq/eSdLLfP86KvrjlPGC5+XVsyUTjm/QFZYaIaiX/vUez+S3kK/S89vVPyBf0Wn+SgqtJ8WJZaBBtcBTRink5MtEfqECRsaogvh7KrR3riAbLwT0IEVcLtmspirfAagZfgdwrGPxqZo5hfnULJxI/V736Y3RHY0YVeA9a+U/y1KXqRMHnUiv7K7EJ/Yam7tDLA8f+6mCovXU9LHM+aoU/CGb7DFeKcHEZWbr1NWJ4w2SN+eseB2+Sz4TUDjye6/7o1Szwbq0L/wY0LqfW43QbuCIdrW3ClbLfLW6I4BLG0dXc2mLCg0F1qgkahBfCpzhskWQn2ptz0shzxwfGDmfVMyqg68o3U0Eq/MwuXTz0ReYG1Eyv+QFn3nqfAkv+voeEfAGbFaDIPpJM4ojiBgmhnlkNGD3ZxJQ3IA5F8L97kIaQAoIB9DyBPVBc1ghuTrDb64fDJ0l6OgcMRAkWO8rf/UzlvcTbdefSwUpbTQDDOZrwjw/jL0LFrRXrCwfHghRDOr9R8bBTjhuac/NnZbUFLh0+xpVY8fAfEWG/A7sfFgFXC5XsVLbJQldnNXjsOYtDXwEh77rGcDwsaRctgnD5gqZc98oimjMGE0EB7RfomT9pqBCsQSl6hfMBXfK1by3leCnrS0Bh5pXbOG9gWHGBpWFbnyCkkj0gVOhWebKAVbXxqym5xER2NKMv1Zcwa346/wooVqKquX4Bn3sfwY3sIFr+o/mhC43E0ZyN9yyXRc/NZhtL/K5iLLwl7z/Td54K7RBf1+lrnDsH5M67wg8klv9tMBgftdHEm28zzMkWK75zO0ICaLBkJgOlQQlh55npdo DPv5yFAA aHrsGLBGC+Ls8lQmI1oUJGzuM73nH2glctvO1pPkSGUnx4WXaqKql1AV4E8di52jiwR5vTMJZpvHJMVhtJD9NW3WJQCAoZlnvxCZ+JwrF0UyfFes1XtxTMSl5zLM7M2SR58rk7+1rf9VDx5pustFrj0alCSNEkMjJTPaE86p5Sn+h/VIsrk/klr+6xQ90Fjbpjz8z9zlZ4do9V0z8/FxnDoaKmyIO66g+OUtypWa1fzYtg5i5F3biUVayobPEYUM6++OicUR5J2s/Mq+LR3iAgpU6P6en6Nz4pA3P5GewwSQJmS1l1+cCwISdJv7j5dFo3DvsrDs/hGnfP0o= 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, Jun 27, 2023 at 8:32=E2=80=AFAM Peter Xu 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 > > --- > > 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 =3D 1 << 10, > > FAULT_FLAG_ORIG_PTE_VALID =3D 1 << 11, > > FAULT_FLAG_VMA_LOCK =3D 1 << 12, > > + FAULT_FLAG_LOCK_DROPPED =3D 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 |=3D 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 =3D __folio_lock_killable(folio); > > if (ret) { > > mmap_read_unlock(mm); > > + vmf->flags |=3D FAULT_FLAG_LOCK_DROPPED; > > return VM_FAULT_RETRY; > > } > > } else { > > IIRC we've discussed about this bits in previous version, and the consens= us > 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 gu= p > 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 >