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 9284DEB64DD for ; Tue, 1 Aug 2023 07:40:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 038FA2800E7; Tue, 1 Aug 2023 03:40:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F2A192800C8; Tue, 1 Aug 2023 03:40:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DCB7C2800E7; Tue, 1 Aug 2023 03:40:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id C92762800C8 for ; Tue, 1 Aug 2023 03:40:49 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 9074C1604D9 for ; Tue, 1 Aug 2023 07:40:49 +0000 (UTC) X-FDA: 81074739018.24.85F5631 Received: from mail-qt1-f179.google.com (mail-qt1-f179.google.com [209.85.160.179]) by imf09.hostedemail.com (Postfix) with ESMTP id DE5A4140009 for ; Tue, 1 Aug 2023 07:40:46 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Z4l96ijO; spf=pass (imf09.hostedemail.com: domain of yuzhao@google.com designates 209.85.160.179 as permitted sender) smtp.mailfrom=yuzhao@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=1690875646; 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=v1ckwMzi+FIp2LWpGFkmYasLA7o5jmHuzGuDzFbHRJI=; b=YWeQetxhCJoCnCztvJ5Tvh6YE7BiDLpC8Cs+14UrPOI3QzHYGTNfF/GXut66ZXMDOyRyTz 7F1ZP2i5ePXxIu686HaNXMS/7tmwsSy8hgGFEkVjjXjmJXRM6u/cMDL3ha8BQzwLCtRDlw ZxdgonCZ8D1MwuTdHwe111N52PCbALA= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Z4l96ijO; spf=pass (imf09.hostedemail.com: domain of yuzhao@google.com designates 209.85.160.179 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690875646; a=rsa-sha256; cv=none; b=hF636Iz44ulUiDhfteNTV8aQgTwjgeT/DeRkiW+pqquD6qcRqh1MbnDgJRaLXayzdZPQoJ RwrvtLOl4xFyaopuywRqmwY6AH5kgeM8CEl2GH329kpJu6JQVP/xa7kmsP7aqIcKwFb+lb SvJ/xwB5X2R7jsYk04p958VLBMTK5Gg= Received: by mail-qt1-f179.google.com with SMTP id d75a77b69052e-4036bd4fff1so222001cf.0 for ; Tue, 01 Aug 2023 00:40:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1690875646; x=1691480446; 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=v1ckwMzi+FIp2LWpGFkmYasLA7o5jmHuzGuDzFbHRJI=; b=Z4l96ijOCUV1jCIh7akPgFxUN7n4TGV+4ixXqZrqoMrJu+qGIaelFu/BYkra1Pi/P8 893iEXuQuzDXJUimzaHponHU2RQjDGCaneTVA77mXZ6CnhiN3d9U/g6cvrUoqbDaOKC3 H//HxyTPW566NzLCjsgOKoEn7W5zJrdMl8iE8OedNPmvqZVmyDr7OUFMIwgLiU5+N/m2 y9KYD270Hvq4q2WffLKgxrBLT9bUF2I0dW1faCBavJb2WtWAivOTV6uQo/FU5TJ2wW/U XwWIq3DC+2vaPy0GYXzz20KmvIFM8+ElbJGCMNc9yiD1LKZ2t+AQDqk/FIKJo8lOYCC2 mPGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690875646; x=1691480446; 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=v1ckwMzi+FIp2LWpGFkmYasLA7o5jmHuzGuDzFbHRJI=; b=Ed/wAEevcTLkm76VpviJO4jh4uRha6I9yqqViqQxvPPjpS0/2HxSw3DhS0DglbYfmQ 2D8pz8S/U7+V27amgHbaszvK5K7ihb9ZeFbmIMMvrHmJo1dMwmtK797lI9b0x+LgCT34 Nxq03XpdsJdmuHCpqiJXKnVEveqe9OlpDmec0ZvXgu1s7701HzaLuXrP69NvPQQxql7s OkkzckhB5fUwGGbyde+/OgOIlNZ7KJCQoSHr6xu+WsK+O/xboopSS9CTqeKO2ZWIBCnV rODEsLAXVLInVT/LA5yB0JDLQuXC2JQ79yX5WouLF6KQvA1KXo5EKovcyx1SEi2dmSkb 1W/w== X-Gm-Message-State: ABy/qLab7po1eHSKTcX5OoeAmBoB0t0PEEEAUlbMFQw+VveVd6LsOCx4 PjxDnc+PHYwPw1uAzYpJVmYIy1zVD6ufj0ip4sUi5Q== X-Google-Smtp-Source: APBJJlE/z1yYLYt6LNaoji+ukmicGnkFXZ2bSaBTlYLoWNAFANjvUdHA7+uDUY1fMUI4yexT4v3uMHavY/A8llix6BA= X-Received: by 2002:a05:622a:1827:b0:403:eeb9:a76 with SMTP id t39-20020a05622a182700b00403eeb90a76mr628181qtc.17.1690875645932; Tue, 01 Aug 2023 00:40:45 -0700 (PDT) MIME-Version: 1.0 References: <20230720112955.643283-1-ryan.roberts@arm.com> <20230720112955.643283-3-ryan.roberts@arm.com> <87r0ouw39n.fsf@yhuang6-desk2.ccr.corp.intel.com> <3c26aa4e-fe11-09d2-c2fb-63546ba80893@arm.com> In-Reply-To: <3c26aa4e-fe11-09d2-c2fb-63546ba80893@arm.com> From: Yu Zhao Date: Tue, 1 Aug 2023 01:40:09 -0600 Message-ID: Subject: Re: [PATCH v3 2/3] mm: Implement folio_remove_rmap_range() To: Ryan Roberts Cc: Matthew Wilcox , "Huang, Ying" , Andrew Morton , Yin Fengwei , David Hildenbrand , Yang Shi , Zi Yan , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: DE5A4140009 X-Rspam-User: X-Stat-Signature: t3zoe5o553cnyjrams5rpoz34sa56oog X-Rspamd-Server: rspam01 X-HE-Tag: 1690875646-294512 X-HE-Meta: U2FsdGVkX1/C5+3diH547sABlcefqh2pk4h7ev5ATO9QhypK5K98n9FRoOL2uWNwuDvMJs90TOcpp3GCPm/+WrhCZxO8sEzc+vQ2qWMtYPw9IpUUQPbC3bYi4YxkUfHlBFirDq81aEicMoCHtzrKp+1ZPehYE1+/C6U1ZTBGgWIpISO1yOX07FyKJFXKpxihkIOnFhTFRUxMC0UQltziI19Lt0o7lICbpPWNaqtTM1mDDrSqM1kGMn4cBsAic84YbUBsbKk1a84uay+PgqAh8RRqucZzXqkwmHLTePXG9zqM/nobeLybM/FnHvNjE291DBAOMg3EBOWw9VV4mGKW2KgzmoAis60b01YuCOOU5BbAnx2fgigXGBEnD/7030DFbyBmcG43T+dnY/5YcunEGgnh7pnn6vOt5vTjodziqRvcHxpU03d+TX/dYNQWQHFn5Yl44dJVOtubfKL/XZwOPlpvm+3abF23gN/w65C07wI46ncy0vKBBG4Rvimj+H3SGpyLvRLHSzrysk221Ds1jU1oBGBu0O9U3Ag6/8usnjbLaCc+aLSxXtiRE50sNDKjaEkYfcQ6zTe9/3pNHt60s0lHty4TZXTTvOpH4GE+hh5HBj43ae60hi4owhfeOH3UDg6311xIDAYcipJKA9W7wK/ZwdD02RQwP3ohQRKXwfxFvVyr8NjcnI/kbFHDaHclTX+ycatxneGvdHlc58ZbP2h8B/DgnInHekYmT2q4WMvRdz5SxBu3xdv/ZdPmm/Yk5OZf0T0HeELg/Wn+eAcaqC/8s04lcWN9MM543X4lsfLNd4UouBwSIO2XMTdzULduhfZk5soJo4zxTW5UewVj83HXfNRSdbAa84WMmvuGBxbRW7KmDZ20LkCyF1q5ge0wCa7v0yGIN5gnlHj6jEp28v8olcc7lHYRjiaAgC/hribgGat6oBZ0zADP6oukI3sF2zRzQ6gHP/bLX233k7Q O0vtDRTG gLDXCbyPg/17mvJbaDh2LW57LD0bLqBtvDucK+A3/sFhtRxcIdm6V8/wsjV0ZrAmekplqKsEx7lZE//NwhW7zhPqMYXI+s80sZD/VzWRXzX4b36afkiTzsB3TYyjy9K4SnGMHrEFF4x31rsVI7RVgI9aLzwkLyRJP9m23d9kQzhcj3UQX2WdWab79F30WOAylAIRJj3+hwJIKAOiAeMErgJm1saOyynaRii0bBIzHUKsA7zvKefUCtCavAAp02L5n7lLYwof/2yBtzltoSgAgk48FQyVMidTGKGeo0b8EEIHMx/IBUVDMGNqpDHVAvsnjOSRkRwAYrrKYU2SehC/xQWwo5VrMOBdSVvU/9Yswp5WCJF6QNDbBJ/TNTwr5M6NcK5/C9g695dIlihY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Jul 28, 2023 at 3:00=E2=80=AFAM Ryan Roberts = wrote: > > On 27/07/2023 17:38, Yu Zhao wrote: > > On Thu, Jul 27, 2023 at 1:26=E2=80=AFAM Ryan Roberts wrote: > >> > >> On 27/07/2023 03:35, Matthew Wilcox wrote: > >>> On Thu, Jul 27, 2023 at 09:29:24AM +0800, Huang, Ying wrote: > >>>> Matthew Wilcox writes: > >>>>> I think that can make sense. Because we limit to a single page tab= le, > >>>>> specifying 'nr =3D 1 << PMD_ORDER' is the same as 'compound =3D tru= e'. > >>>>> Just make it folio, page, nr, vma. I'd actually prefer it as (vma, > >>>>> folio, page, nr), but that isn't the convention we've had in rmap u= p > >>>>> until now. > >>>> > >>>> IIUC, even if 'nr =3D 1 << PMD_ORDER', we may remove one PMD 'compou= nd' > >>>> mapping, or 'nr' PTE mapping. So, we will still need 'compound' (or > >>>> some better name) as parameter. > >>> > >>> Oh, this is removing ... so you're concerned with the case where we'v= e > >>> split the PMD into PTEs, but all the PTEs are still present in a sing= le > >>> page table? OK, I don't have a good answer to that. Maybe that torp= edoes > >>> the whole idea; I'll think about it. > >> > >> This is exactly why I think the approach I've already taken is the cor= rect one; > >> a 'range' makes no sense when you are dealing with 'compound' pages be= cause you > >> are accounting the entire folio. So surely its better to reflect that = by only > >> accounting small pages in the range version of the API. > > > > If the argument is the compound case is a separate one, then why not a > > separate API for it? > > > > I don't really care about whether we think 'range' makes sense for > > 'compound' or not. What I'm saying is: > > 1. if they are considered one general case, then one API with the > > compound parameter. > > 2. if they are considered two specific cases, there should be two APIs. > > This common design pattern is cleaner IMO. > > Option 2 definitely makes sense to me and I agree that it would be cleane= r to > have 2 separate APIs, one for small-page accounting (which can accept a r= ange > within a folio) and one for large-page accounting (i.e. compound=3Dtrue i= n today's > API). > > But... > > 1) That's not how the rest of the rmap API does it Yes, but that's how we convert things: one step a time. > 2) This would be a much bigger change since I'm removing an existing API = and > replacing it with a completely new one (there are ~20 call sites to fix u= p). I > was trying to keep the change small and manageable by maintaining the cur= rent > API but moving all the small-page logic to the new API, so the old API is= a > wrapper in that case. I don't get how it'd be "much bigger". Isn't it just a straightforward replacement? > 3) You would also need an API for the hugetlb case, which page_remove_rma= p() > handles today. Perhaps that could also be done by the new API that handle= s the > compound case. But then you are mixing and matching your API styles - one= caters > for 1 specific case, and the other caters for 2 cases and figures out whi= ch one. You are talking about cases *inside* the APIs, and that's irrelevant to the number of APIs: we only need two -- one supports a range within a folio and the other takes a folio as a single unit. > > Right now we have an overlap (redundancy) -- people would have to do > > two code searches: one for page_remove_rmap() and the other for > > folio_remove_rmap_range(nr=3D1), and this IMO is a bad design pattern. > > I'm open to doing the work to remove this redundancy, but I'd like to hea= r > concensus on this thread that its the right approach first. Although pers= onally > I don't see a problem with what I've already done; If you want to operate= on a > page (inc the old concept of a "compound page" and a hugetlb page) call t= he old > one. If you want to operate on a range of pages in a folio, call the new = one.