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 7F170C25B76 for ; Wed, 5 Jun 2024 23:22:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 12D656B00A6; Wed, 5 Jun 2024 19:22:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0DD666B00A7; Wed, 5 Jun 2024 19:22:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC0576B00A8; Wed, 5 Jun 2024 19:22:41 -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 CBBA06B00A6 for ; Wed, 5 Jun 2024 19:22:41 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 84981C116B for ; Wed, 5 Jun 2024 23:22:41 +0000 (UTC) X-FDA: 82198411722.26.18D2B83 Received: from mail-yb1-f177.google.com (mail-yb1-f177.google.com [209.85.219.177]) by imf18.hostedemail.com (Postfix) with ESMTP id C412D1C0014 for ; Wed, 5 Jun 2024 23:22:39 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=M+IKzSbr; spf=pass (imf18.hostedemail.com: domain of surenb@google.com designates 209.85.219.177 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=1717629759; a=rsa-sha256; cv=none; b=rjD/P5QmsVbfulrEeBEK20PIzmbrQ1h4mPBo+GJkLMCL1xbCsLoEJgLldVcivhxYPPIUBd NNPXREWeKynLTnnpEQvO9x45capErT2jYJvgA2QmkX3PlsQJjq0g5lsgnZqacNvaRzKXzA YkiLCSFUAR3yoKJgQfidwz51VtrFxj0= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=M+IKzSbr; spf=pass (imf18.hostedemail.com: domain of surenb@google.com designates 209.85.219.177 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=1717629759; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=Cw47IidnP2/eaju7UvZBzqOetg82MP8FWmgrm1mdRb4=; b=e86uvqpvDWud4zDrMo88HkyiKD5vFqmAB+Ey9jw+OcowvcX7mkJvfTg5OY5Shdp01qhQ9S yjkFSN0QbTwKcxh6rnP2cjaO5UevytHP283VdJPpT0Bc5yDeVyzoqe9leNYmJIq4FWnrhB YVrzS8f5ZYiNwA7cV2cfdEtJaayxxqM= Received: by mail-yb1-f177.google.com with SMTP id 3f1490d57ef6-df4d5d0b8d0so370306276.2 for ; Wed, 05 Jun 2024 16:22:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1717629759; x=1718234559; darn=kvack.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Cw47IidnP2/eaju7UvZBzqOetg82MP8FWmgrm1mdRb4=; b=M+IKzSbrMnpbwc/OYotmQlE+X2cRGczZwcezD/rmY2B/OPNDbtGfJaCfBesqYTnzKT L39uRPZys7NIi/RFxZb37WdA/YquXGtZe75g0X5oM975wHu7Gf65xPiVSIv+mwNE6aPU 9e93v7E5MfpPrG6/+t9SMTquw7j4vgON6EQGvj9bysjUSXPns6pxfTR1Df5Jnjplr//o yktNAVEuUS3Ogu9LBHgnM/HCq1WPsmpEudKNqYGU1zOyOB4uuE5SWDSqejVT2MosLGFf CTA055k+V9Fx+9IBwnD/RcMmNe/ZZ5cZ8Z26k5xxBfX9kOVj5XtDrSbbK5s2iLWfqMQa s4NA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717629759; x=1718234559; h=content-transfer-encoding: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=Cw47IidnP2/eaju7UvZBzqOetg82MP8FWmgrm1mdRb4=; b=E85b/uzDB94PndkkvAyWxjQCl06Lwm9DMysQPTLh6txP2uwF+FN/5/lIHaG2oSU5wb MDoGPsYr7z4F6Bud6s5rwKecDetBRtJPkbCFA4ppmcSsEAEH2lEVm2Zsa9uGfXUhnaWQ +aX4xP12dofAQ7VlOkoG0REH01lwHXtR+oLtjgO+OMVJvRt/pGqk+RnoLGLJ+MHF/RGW /OPJDVMXFJBFlYLcbb89x5xNqN8FKwVkdf9dImtwF4V40zHvXcVv7N53D03ihpvLRyLe oyJqhhdetkU8LXqOZtBJM3HNjvnfdbM3va5Tjm3cnUQPuv4KjgUKfZ5Gdd8NB7LOfztO eK4Q== X-Forwarded-Encrypted: i=1; AJvYcCU/XoIXyeFflu9TcAC411hMnLMucjevmCAGGwov7rgafFKf+tUJGy1ElYeJMbclFDda7mgeK2ALRNg6bry88IkvhBc= X-Gm-Message-State: AOJu0Yy9XIyXmmmqt92dSgk4tBdmycaGrHUKztXIovsXYBSnoO+ha9gz L0zLqXcvtgQaoT5/AvLaM8OihBPMx7X5UFtECD/pOQ0Ugf1E5jcRDrgJy4zvVx0kiIuPJpvb7iw nIwSUWXIudBeuV9cSPL4UrlxLjlNpT5fENelj X-Google-Smtp-Source: AGHT+IHrmFvJ9bp9T7DdePm64tfrOr9ZbqYnpVOs/J0OmO5DpmHefsY4GHCM70DHHhs4RMyXVR80eW0h81Jh5Fi+/5A= X-Received: by 2002:a05:6902:1793:b0:df7:923f:f2a4 with SMTP id 3f1490d57ef6-dfacab2c208mr4276890276.7.1717629758331; Wed, 05 Jun 2024 16:22:38 -0700 (PDT) MIME-Version: 1.0 References: <20240605002459.4091285-1-andrii@kernel.org> <20240605002459.4091285-2-andrii@kernel.org> <5fmylram4hhrrdl7vf6odyvuxcrvhipsx2ij5z4dsfciuzf4on@qwk7qzze6gbt> In-Reply-To: <5fmylram4hhrrdl7vf6odyvuxcrvhipsx2ij5z4dsfciuzf4on@qwk7qzze6gbt> From: Suren Baghdasaryan Date: Wed, 5 Jun 2024 16:22:27 -0700 Message-ID: Subject: Re: [PATCH v3 1/9] mm: add find_vma()-like API but RCU protected and taking VMA lock To: "Liam R. Howlett" , Andrii Nakryiko , Matthew Wilcox , Andrii Nakryiko , linux-fsdevel@vger.kernel.org, brauner@kernel.org, viro@zeniv.linux.org.uk, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, gregkh@linuxfoundation.org, linux-mm@kvack.org, surenb@google.com, rppt@kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: C412D1C0014 X-Stat-Signature: agttqbajuiegbi38e3z45or3qpxxrwmj X-HE-Tag: 1717629759-492009 X-HE-Meta: U2FsdGVkX1+5c2rjTA3zEvUAx0FuNnlYRSNq5BZrba0NjDksEfq4SuRMckd7qCHvr+46MIexgW1ocf1aL8+PHQCpLKtq45gBUKBjRMMKlYyrdcr39EDtPHdavMhI+rj0vaHcXqsNGVXz0K8bBXjCL0cAIdp0DT+8yV7dZVReQMU6l+RWe2stZvzruYoRBp5Eckw7666uXVVlUA2v5z+WMREhfJkp/yYC7h/6kYwZwZ2ZMA1q0hQ5Jq6HlNJWyY1jeEY5txXMc0Fcg/4tX5ojLEpc7bsc9rTME2PLc0tPCfE6mXavGsI4lnhwvJXzBtF5yXisVW8gcN6svTvgnoETRmYj16eF0ESct7KhEyHlVd2iO8RCakoAV+2zfQrzWyKrCQ+iZra/a0ZEX35I9uY87F8UAVrRzgxi8yquSi7bwwOqJUWHinM1tlGsEarK4JsizkjXzyokyyPmPxTBwfoTRpzvYtXKPbS71Infw0Ma6JPfE4MnCFeppIHmJhRsWohpIFMxpPjnc2m/oGJU622sdKPJOXwulmCw4xuEWbAx0ZcNBGv8SiAcRKBmon6BNCFdQk5PwJg4g1zdnZHM3bAunjO74Lon1v0pv2lPXq9PgcO/gAzvRqk/aaEn6SxVYPQlk7bUBiUaQBEjkHWe7TBQiKmKM47TbYhkTJcwOiFzoPM56ppYPhe0sSeFY41JLIcmEBSGs9T9gMTAOas63bIKpQTe7kFmwjX2dL17OuExIb6+7D5Zkc+1iE8jrHugS1yRbiPpJsCFf0vJYFOo6NoMsuI0kyEi6DQOVGItuBueos53CcFCpAVVvCVzbteQIsGnthcimb/quYrfD0pJBElnx16PviRgrNTHDZMB9gAMaKHE7/ohMCYH5uNsjSN71ISMnpUS4ndr3n2nub784o2UAUAswarewbquPvICd/7zttsXG0l5K2VdnRi07vsFVmSlez98HNK6OjmA8Az5i7p wIzg/L7b CpSp5LKV4v18g0E9N3jLlndgqqQPLR+YxJ7bby+4yLzh3HuAuZRhDJq6HXSb7jiwdqo4guJgpK46AWGanf4o5oWIisPwT05Ff08k0D5Son135/XZd7WKFlrdxd4tKa9eeKpAFpqReXnOM+Jd0AoqS/ZRg6aQEgHUlbmBJgspR+yGBkdapvJvPu3Xiz/PK8yuC6v0UiJX+G+IscLNppab/bFOgvn0cxdjEZ0IXWGE+voG7JxPruMJAnL/SlqPlnEANUrFRfTrwGBG6JuO17TIndjGgxG4VSivm/SFo/AhxdmS8AnH9QR3yXZ6Gk0XNvv61aFkmLE+iyiYWCfIEAT+ZDpGwOcQfZmsqsDE8UbJdf4Dbv4D/yNhvMlb0ZPeaUnYvPG7vCzBeyABZaIPK2W5DzqG9R2aSisd2hXq8J8aO8fASPhux3VqW5QxeqyqgZj6O2ymQTXvygENgH00= 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: List-Subscribe: List-Unsubscribe: On Wed, Jun 5, 2024 at 10:03=E2=80=AFAM Liam R. Howlett wrote: > > * Andrii Nakryiko [240605 12:27]: > > On Wed, Jun 5, 2024 at 9:24=E2=80=AFAM Andrii Nakryiko > > wrote: > > > > > > On Wed, Jun 5, 2024 at 9:13=E2=80=AFAM Andrii Nakryiko > > > wrote: > > > > > > > > On Wed, Jun 5, 2024 at 6:33=E2=80=AFAM Liam R. Howlett wrote: > > > > > > > > > > * Matthew Wilcox [240604 20:57]: > > > > > > On Tue, Jun 04, 2024 at 05:24:46PM -0700, Andrii Nakryiko wrote= : > > > > > > > +/* > > > > > > > + * find_and_lock_vma_rcu() - Find and lock the VMA for a giv= en address, or the > > > > > > > + * next VMA. Search is done under RCU protection, without ta= king or assuming > > > > > > > + * mmap_lock. Returned VMA is guaranteed to be stable and no= t isolated. > > > > > > > > > > > > You know this is supposed to be the _short_ description, right? > > > > > > Three lines is way too long. The full description goes between= the > > > > > > arguments and the Return: line. > > > > > > > > Sure, I'll adjust. > > > > > > > > > > > > > > > > > + * @mm: The mm_struct to check > > > > > > > + * @addr: The address > > > > > > > + * > > > > > > > + * Returns: The VMA associated with addr, or the next VMA. > > > > > > > + * May return %NULL in the case of no VMA at addr or above. > > > > > > > + * If the VMA is being modified and can't be locked, -EBUSY = is returned. > > > > > > > + */ > > > > > > > +struct vm_area_struct *find_and_lock_vma_rcu(struct mm_struc= t *mm, > > > > > > > + unsigned long addres= s) > > > > > > > +{ > > > > > > > + MA_STATE(mas, &mm->mm_mt, address, address); > > > > > > > + struct vm_area_struct *vma; > > > > > > > + int err; > > > > > > > + > > > > > > > + rcu_read_lock(); > > > > > > > +retry: > > > > > > > + vma =3D mas_find(&mas, ULONG_MAX); > > > > > > > + if (!vma) { > > > > > > > + err =3D 0; /* no VMA, return NULL */ > > > > > > > + goto inval; > > > > > > > + } > > > > > > > + > > > > > > > + if (!vma_start_read(vma)) { > > > > > > > + err =3D -EBUSY; > > > > > > > + goto inval; > > > > > > > + } > > > > > > > + > > > > > > > + /* > > > > > > > + * Check since vm_start/vm_end might change before we loc= k the VMA. > > > > > > > + * Note, unlike lock_vma_under_rcu() we are searching for= VMA covering > > > > > > > + * address or the next one, so we only make sure VMA wasn= 't updated to > > > > > > > + * end before the address. > > > > > > > + */ > > > > > > > + if (unlikely(vma->vm_end <=3D address)) { > > > > > > > + err =3D -EBUSY; > > > > > > > + goto inval_end_read; > > > > > > > + } > > > > > > > + > > > > > > > + /* Check if the VMA got isolated after we found it */ > > > > > > > + if (vma->detached) { > > > > > > > + vma_end_read(vma); > > > > > > > + count_vm_vma_lock_event(VMA_LOCK_MISS); > > > > > > > + /* The area was replaced with another one */ > > > > > > > > > > > > Surely you need to mas_reset() before you goto retry? > > > > > > > > > > Probably more than that. We've found and may have adjusted the > > > > > index/last; we should reconfigure the maple state. You should pr= obably > > > > > use mas_set(), which will reset the maple state and set the index= and > > > > > long to address. > > > > > > > > Yep, makes sense, thanks. As for the `unlikely(vma->vm_end <=3D > > > > address)` case, I presume we want to do the same, right? Basically,= on > > > > each retry start from the `address` unconditionally, no matter what= 's > > > > the reason for retry. > > > > > > ah, never mind, we don't retry in that situation, I'll just put > > > `mas_set(&mas, address);` right before `goto retry;`. Unless we shoul= d > > > actually retry in the case when VMA got moved before the requested > > > address, not sure, let me know what you think. Presumably retrying > > > will allow us to get the correct VMA without the need to fall back to > > > mmap_lock? > > > > sorry, one more question as I look some more around this (unfamiliar > > to me) piece of code. I see that lock_vma_under_rcu counts > > VMA_LOCK_MISS on retry, but I see that there is actually a > > VMA_LOCK_RETRY stat as well. Any reason it's a MISS instead of RETRY? > > Should I use MISS as well, or actually count a RETRY? > > > > VMA_LOCK_MISS is used here because we missed the VMA due to a write > happening to move the vma (rather rare). The VMA_LOCK missed the vma. > > VMA_LOCK_RETRY is used to indicate we need to retry under the mmap lock. > A retry is needed after the VMA_LOCK did not work under rcu locking. Originally lock_vma_under_rcu() was used only inside page fault path, so these counters helped us quantify how effective VMA locking is when handling page faults. With more users of that function these counters will be affected by other paths as well. I'm not sure but I think it makes sense to use them only inside page fault path, IOW we should probably move count_vm_vma_lock_event() calls outside of lock_vma_under_rcu() and add them only when handling page faults. > > Thanks, > Liam