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 F2284C67861 for ; Tue, 9 Apr 2024 10:30:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 530086B0082; Tue, 9 Apr 2024 06:30:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E0466B0083; Tue, 9 Apr 2024 06:30:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 381266B0087; Tue, 9 Apr 2024 06:30:07 -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 1A7C36B0082 for ; Tue, 9 Apr 2024 06:30:07 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id BB7621A0326 for ; Tue, 9 Apr 2024 10:30:06 +0000 (UTC) X-FDA: 81989623212.05.424D808 Received: from mail-vs1-f52.google.com (mail-vs1-f52.google.com [209.85.217.52]) by imf18.hostedemail.com (Postfix) with ESMTP id EEFCA1C000E for ; Tue, 9 Apr 2024 10:30:04 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=DaD+Gi3t; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712658605; 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=dsGoKaeRkKgjEDjNb3s/lpnka/plaKuqGEcl6UDIdVU=; b=2IoXvf42tuD86tzFc25YAs0nxglfhGsBP/WYjxFVkiHsPTeGYRnUs5zqbu4Y8NI07AwmKJ 6DFJGqJKBqUxOMLH+gQhnsLowLWKZxOMyjCraVjP+ePQaLx6W915lP4rOKxbT55frBi8CH ytwkl3X+j40Rri9qrIg0H/TUJZyWJEc= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=DaD+Gi3t; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf18.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.52 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712658605; a=rsa-sha256; cv=none; b=efi63NY1CcFSddhsaoG5lLQkiRiYkdVdJka9VVF9FsmRQDcXdAmHESW+FNkIM3Il26xFgL 6ybefCR/MgJy1Cy1jPkBkvyijwyDpMVNkYg5Ly+W+z1cLifRdIfLi8ptwjazN0zKFnmwYR dCQLZ9G65DUIkmQRlR2dRYUEZQXT/4M= Received: by mail-vs1-f52.google.com with SMTP id ada2fe7eead31-479e45cc9e7so1230075137.3 for ; Tue, 09 Apr 2024 03:30:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712658604; x=1713263404; 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=dsGoKaeRkKgjEDjNb3s/lpnka/plaKuqGEcl6UDIdVU=; b=DaD+Gi3tO7AN5oi6i6IgQ2FxwkCEqczrZBBpQrA2Z1V7XAK8PD1Wl3aTMDGS3Pvbaw tnKgR3d+mfAKZ8imrTjnMuaFzKAXF4YGGdx40iFQYNln5Zwo1qFyU69oJkxVXkGdosHY GiGFJ84xm68z2XRgKRL4pQfBZY/NaeOpp6HArZl11Fzura0IeeLDzK2MyO27eWF4EG8X 1X/2r2AvyqXCQycGAcdQ56ursQlFNuDEl79tHOkr79xfFjj7Cdl0VBGcNggozdq56qUF 2Ys/y9sJHZwZ7iM4rsW1dJxVW9BqqUIfUQHb066WnW9KdI8uww7HIwdWvRZMKnDdHtXd P8Qg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712658604; x=1713263404; 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=dsGoKaeRkKgjEDjNb3s/lpnka/plaKuqGEcl6UDIdVU=; b=tp2nECx7ORbagEhfpV5FlYmWJpv6vgnRbbfAdcADSoKImbag/sLi0hCGWWUtfrln1b kKcp9q2456r7fg4FEIxEgSMkHKDoHjxT3EBkClvvVzFejTAb27jQkHXJomr/L0boI9I7 lK9c4cTOpSm9A78m2YDdxK8oMHOVJSWUQYtJX8lNGGL2Bn8OxE+ACaLfk9ohCc3srLRo v44KZnPdGnKt7vVGcGxd+4WSZRUIZ/MGqhunUPzJBfTLOGajmAR04o4HJBsO7BrFZHxY sMv4Kh2X9wGXCAKr1CiYleacsdXPmB9+2AhMHwQauwMr4YNpQpmo76UdraCm2WJ28GYh QbQQ== X-Forwarded-Encrypted: i=1; AJvYcCXG71QFaInINvUno5x74pIGRUJF0l135F3tx6+CBtSUlhSmH045xdVsRw2q+8ACgfvhUuleqwO7z7DPETGgZv9ttfk= X-Gm-Message-State: AOJu0YzLpl7LiScdJfTnU9beoLekDdTxn55gq8OvcdBv+7nTqW17Kydk EXDKmmBDv/LuqDkfIU5P8sngjGUCZy0QhURgHWDzZ61yiJb+OrvSH1mkmYolsdfp6UrMc7CAQwR uKOj6QPuqn+jKcQ5HbIqpE3LfClE= X-Google-Smtp-Source: AGHT+IHosp1ukWRmHjxX7dz7334NnaNrsDrOU2y6hy8Y2FNjF8zXS0KN8m4FS49RYGMkaOw0/alNyvfG/SWeJVZysE0= X-Received: by 2002:a05:6102:3f06:b0:475:ffc3:3b3e with SMTP id k6-20020a0561023f0600b00475ffc33b3emr12261655vsv.29.1712658603916; Tue, 09 Apr 2024 03:30:03 -0700 (PDT) MIME-Version: 1.0 References: <20240408183946.2991168-1-ryan.roberts@arm.com> <20240408183946.2991168-3-ryan.roberts@arm.com> <44e23a4d-97e7-4d84-848a-f9325b7b76c0@redhat.com> In-Reply-To: From: Barry Song <21cnbao@gmail.com> Date: Tue, 9 Apr 2024 22:29:52 +1200 Message-ID: Subject: Re: [PATCH v7 2/7] mm: swap: free_swap_and_cache_nr() as batched free_swap_and_cache() To: Ryan Roberts Cc: David Hildenbrand , Andrew Morton , Matthew Wilcox , Huang Ying , Gao Xiang , Yu Zhao , Yang Shi , Michal Hocko , Kefeng Wang , Chris Li , Lance Yang , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: EEFCA1C000E X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: ey97ge1xm3o4fp6z8rx3mh9tprikop6h X-HE-Tag: 1712658604-124503 X-HE-Meta: U2FsdGVkX18xOkJ8q8TYDfz1yDgpLXgrW20KgRfovDqF7GA/4oCOm9gAIllO5L8DZfpn+WtXhDxGO1ot2bMds8IXhdWhS/ZssbI79VCMbP4TzuVdN5hLmlpi4zbzHSVwLPh4iViBZVKqzQO0oMLoMla9Dny05ojOa63FAIhl9YWkJzc/ue5hE8gZ2uf6Aldk/6AcoNaPBvgO2hOTo2gtPLni7qEWvdEC1Wk8KmD23FvhZNLA2gyQ/goPBgc+75aMMEvAYjt1cCDGYFlhnAC2Au7Vq5pzZc7Fv6qMEAz9pRZStcm0eJgI1ibAsoKrqGJj693x9ZUGudkAn/99A7oTvEOdvLQAsnK2tqZF9NUaTm70YK2ZkVz0TNbab54bxYTrMIwOrRrNGqGlh7J5FTuim7rZsI+m0A4fQT2mpUTZA//zxfYx2rFydURzYYrnHYRkniuMH+C0pFGAM/fHiFi4beZnz7w6sjWWYNRFZfPYxZG1Yd2SXXm3mUrFj4Rdn4mYsAIQiQRHv9qWkng6V6IyEQOA1HWRlUFTMTXJU4qyMAkVvn742xKHpq2z1j3vhy8K3MdwieXtLAZF4LPF3BgoVs62eM5HQOXKLF/6ws1JLin7C95A/2cT4LvbSLkylPoPXrdvRkCOxRfgd0ncTrCuKGLNRH5g/cjji9KXA2IcW1zVAJaFnl9m8pbWe5h7VcONQ3WJcTAjJPZ+prPs5ULB07dLkKgivzV92ikr54KFxRky9Ll5ZWG+wqIhzNG4927avZIBpaAd54ah34Goz4/CGxrYUH1Jbd7Yz6+a7IcM4wGJsCAze0fxRZs4KBrVVrVBzW2MsA7EAuMMwkljPk4b7x/Nzgnil9qs9a69wncqHYhgaHLneQqZlzWVHCKT9TMKaBMSkyCZVXfmyfDCMCwA3h56wpOrwxtuSAu/UnBOtBnLmJf26HUdKR0o+jyXoGSldOTeuX+LMGIgSutdaTn mgvIWTG1 hxjAyWpHYKGnf/xN/mweOYQoEsSIfbXIuwyllcu9gA9d088AsKdWXy5KpcalDRCNvzBPURuzScbNRBlK7+qZtM2tKm3KqQ6/jy0TS6TvmMZ0YgqdvVO04InHCiWlcwpmBJy2BolRnh6G/QiB99Sg1NG91134R6UtrJuWjf6kgd1fiuB96Y1S+CXa+Wd4nNMcNbUErnnL6gnhXkrX3tOa12b4FrrAt4kOmEPMUt04me/e/LJQuvFy1tnrZ8D5P/rEKdwJahsdDpl9T8YH6J+ig9848g6hjYNdOaVvlpNySIPYo0OUmEftZH0CBUK4NXpIO3CAVbVDLcGRbK0VXo+BXA/s2BAxeG/h/ZSTGvtYPJPMICDqUfD2ybSs+5U+3uib0kaoqSQGhyuFBmdHMlQpojiPzWWvYYIlOj+IKuXe0D00CpsrhGlZNn3JoByGTNNrDGquo30B8fy/B0HmClWQdf/cOBZ9l+VH9CSL68ti4ZseBn9RG7YIAXvZSfRJHRNdRoH7O8Y35xnX/H/WZZX4f0mihCttageCBOEoRjuNfE+dHsSoj++MzR2/pUaThdARJLysy 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 Tue, Apr 9, 2024 at 9:55=E2=80=AFPM Ryan Roberts = wrote: > > On 09/04/2024 10:41, Barry Song wrote: > > On Tue, Apr 9, 2024 at 9:24=E2=80=AFPM David Hildenbrand wrote: > >> > >> On 09.04.24 11:22, Barry Song wrote: > >>> On Tue, Apr 9, 2024 at 8:51=E2=80=AFPM Barry Song <21cnbao@gmail.com>= wrote: > >>>> > >>>> On Tue, Apr 9, 2024 at 6:40=E2=80=AFAM Ryan Roberts wrote: > >>>>> > >>>>> Now that we no longer have a convenient flag in the cluster to dete= rmine > >>>>> if a folio is large, free_swap_and_cache() will take a reference an= d > >>>>> lock a large folio much more often, which could lead to contention = and > >>>>> (e.g.) failure to split large folios, etc. > >>>>> > >>>>> Let's solve that problem by batch freeing swap and cache with a new > >>>>> function, free_swap_and_cache_nr(), to free a contiguous range of s= wap > >>>>> entries together. This allows us to first drop a reference to each = swap > >>>>> slot before we try to release the cache folio. This means we only t= ry to > >>>>> release the folio once, only taking the reference and lock once - m= uch > >>>>> better than the previous 512 times for the 2M THP case. > >>>>> > >>>>> Contiguous swap entries are gathered in zap_pte_range() and > >>>>> madvise_free_pte_range() in a similar way to how present ptes are > >>>>> already gathered in zap_pte_range(). > >>>>> > >>>>> While we are at it, let's simplify by converting the return type of= both > >>>>> functions to void. The return value was used only by zap_pte_range(= ) to > >>>>> print a bad pte, and was ignored by everyone else, so the extra > >>>>> reporting wasn't exactly guaranteed. We will still get the warning = with > >>>>> most of the information from get_swap_device(). With the batch vers= ion, > >>>>> we wouldn't know which pte was bad anyway so could print the wrong = one. > >>>>> > >>>>> Signed-off-by: Ryan Roberts > >>>>> --- > >>>>> include/linux/pgtable.h | 29 ++++++++++++ > >>>>> include/linux/swap.h | 12 +++-- > >>>>> mm/internal.h | 63 ++++++++++++++++++++++++++ > >>>>> mm/madvise.c | 12 +++-- > >>>>> mm/memory.c | 13 +++--- > >>>>> mm/swapfile.c | 97 +++++++++++++++++++++++++++++++++---= ----- > >>>>> 6 files changed, 195 insertions(+), 31 deletions(-) > >>>>> > >>>>> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > >>>>> index a3fc8150b047..75096025fe52 100644 > >>>>> --- a/include/linux/pgtable.h > >>>>> +++ b/include/linux/pgtable.h > >>>>> @@ -708,6 +708,35 @@ static inline void pte_clear_not_present_full(= struct mm_struct *mm, > >>>>> } > >>>>> #endif > >>>>> > >>>>> +#ifndef clear_not_present_full_ptes > >>>>> +/** > >>>>> + * clear_not_present_full_ptes - Clear multiple not present PTEs w= hich are > >>>>> + * consecutive in the pgtable. > >>>>> + * @mm: Address space the ptes represent. > >>>>> + * @addr: Address of the first pte. > >>>>> + * @ptep: Page table pointer for the first entry. > >>>>> + * @nr: Number of entries to clear. > >>>>> + * @full: Whether we are clearing a full mm. > >>>>> + * > >>>>> + * May be overridden by the architecture; otherwise, implemented a= s a simple > >>>>> + * loop over pte_clear_not_present_full(). > >>>>> + * > >>>>> + * Context: The caller holds the page table lock. The PTEs are al= l not present. > >>>>> + * The PTEs are all in the same PMD. > >>>>> + */ > >>>>> +static inline void clear_not_present_full_ptes(struct mm_struct *m= m, > >>>>> + unsigned long addr, pte_t *ptep, unsigned int nr, i= nt full) > >>>>> +{ > >>>>> + for (;;) { > >>>>> + pte_clear_not_present_full(mm, addr, ptep, full); > >>>>> + if (--nr =3D=3D 0) > >>>>> + break; > >>>>> + ptep++; > >>>>> + addr +=3D PAGE_SIZE; > >>>>> + } > >>>>> +} > >>>>> +#endif > >>>>> + > >>>>> #ifndef __HAVE_ARCH_PTEP_CLEAR_FLUSH > >>>>> extern pte_t ptep_clear_flush(struct vm_area_struct *vma, > >>>>> unsigned long address, > >>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h > >>>>> index f6f78198f000..5737236dc3ce 100644 > >>>>> --- a/include/linux/swap.h > >>>>> +++ b/include/linux/swap.h > >>>>> @@ -471,7 +471,7 @@ extern int swap_duplicate(swp_entry_t); > >>>>> extern int swapcache_prepare(swp_entry_t); > >>>>> extern void swap_free(swp_entry_t); > >>>>> extern void swapcache_free_entries(swp_entry_t *entries, int n); > >>>>> -extern int free_swap_and_cache(swp_entry_t); > >>>>> +extern void free_swap_and_cache_nr(swp_entry_t entry, int nr); > >>>>> int swap_type_of(dev_t device, sector_t offset); > >>>>> int find_first_swap(dev_t *device); > >>>>> extern unsigned int count_swap_pages(int, int); > >>>>> @@ -520,8 +520,9 @@ static inline void put_swap_device(struct swap_= info_struct *si) > >>>>> #define free_pages_and_swap_cache(pages, nr) \ > >>>>> release_pages((pages), (nr)); > >>>>> > >>>>> -/* used to sanity check ptes in zap_pte_range when CONFIG_SWAP=3D0= */ > >>>>> -#define free_swap_and_cache(e) is_pfn_swap_entry(e) > >>>>> +static inline void free_swap_and_cache_nr(swp_entry_t entry, int n= r) > >>>>> +{ > >>>>> +} > >>>>> > >>>>> static inline void free_swap_cache(struct folio *folio) > >>>>> { > >>>>> @@ -589,6 +590,11 @@ static inline int add_swap_extent(struct swap_= info_struct *sis, > >>>>> } > >>>>> #endif /* CONFIG_SWAP */ > >>>>> > >>>>> +static inline void free_swap_and_cache(swp_entry_t entry) > >>>>> +{ > >>>>> + free_swap_and_cache_nr(entry, 1); > >>>>> +} > >>>>> + > >>>>> #ifdef CONFIG_MEMCG > >>>>> static inline int mem_cgroup_swappiness(struct mem_cgroup *memcg) > >>>>> { > >>>>> diff --git a/mm/internal.h b/mm/internal.h > >>>>> index 3bdc8693b54f..de68705624b0 100644 > >>>>> --- a/mm/internal.h > >>>>> +++ b/mm/internal.h > >>>>> @@ -11,6 +11,8 @@ > >>>>> #include > >>>>> #include > >>>>> #include > >>>>> +#include > >>>>> +#include > >>>>> #include > >>>>> > >>>>> struct folio_batch; > >>>>> @@ -189,6 +191,67 @@ static inline int folio_pte_batch(struct folio= *folio, unsigned long addr, > >>>>> > >>>>> return min(ptep - start_ptep, max_nr); > >>>>> } > >>>>> + > >>>>> +/** > >>>>> + * pte_next_swp_offset - Increment the swap entry offset field of = a swap pte. > >>>>> + * @pte: The initial pte state; is_swap_pte(pte) must be true. > >>>>> + * > >>>>> + * Increments the swap offset, while maintaining all other fields,= including > >>>>> + * swap type, and any swp pte bits. The resulting pte is returned. > >>>>> + */ > >>>>> +static inline pte_t pte_next_swp_offset(pte_t pte) > >>>>> +{ > >>>>> + swp_entry_t entry =3D pte_to_swp_entry(pte); > >>>>> + pte_t new =3D __swp_entry_to_pte(__swp_entry(swp_type(entry= ), > >>>>> + swp_offset(entry= ) + 1)); > >>>>> + > >>>>> + if (pte_swp_soft_dirty(pte)) > >>>>> + new =3D pte_swp_mksoft_dirty(new); > >>>>> + if (pte_swp_exclusive(pte)) > >>>>> + new =3D pte_swp_mkexclusive(new); > >>>>> + if (pte_swp_uffd_wp(pte)) > >>>>> + new =3D pte_swp_mkuffd_wp(new); > >>>> > >>>> I don't quite understand this. If this page table entry is exclusive= , > >>>> will its subsequent page table entry also be exclusive without > >>>> question? > >>>> in try_to_unmap_one, exclusive is per-subpage but not per-folio: > >>>> > >>>> anon_exclusive =3D folio_test_anon(folio) && > >>>> PageAnonExclusive(subpage); > >>>> > >>>> same questions also for diry, wp etc. > >>> > >>> Sorry for the noise. you are right. based on your new version, I thin= k I should > >>> entirely drop: > >>> > >>> [PATCH v2 3/5] mm: swap_pte_batch: add an output argument to reture i= f > >>> all swap entries are exclusive > >> > >> Yes. If we ever want to ignore some bits, we should likely add flags t= o > >> change the behavior, like for folio_pte_batch(). > >> > >> For swapin, you really want the exclusive bits to match, though. > > > > I am not quite sure I definitely need exclusive bits to match. i can ei= ther > > drop my 3/5 or ignore the exclusive bit as below (if anyone is not shar= ed, > > swpin won't reuse the large folio, but it can still entirely map it rea= d-only): > > > > diff --git a/mm/internal.h b/mm/internal.h > > index cae39c372bfc..5726e729c9ee 100644 > > --- a/mm/internal.h > > +++ b/mm/internal.h > > @@ -253,10 +253,22 @@ static inline int swap_pte_batch(pte_t > > *start_ptep, int max_nr, pte_t pte, > > *any_shared |=3D !pte_swp_exclusive(pte); > > > > while (ptep < end_ptep) { > > + pte_t ignore_exclusive_pte; > > + pte_t ignore_exclusive_expected_pte; > > pte =3D ptep_get(ptep); > > > > - if (!pte_same(pte, expected_pte)) > > - break; > > + if (any_shared) { > > + ignore_exclusive_pte =3D pte; > > + ignore_exclusive_expected_pte =3D expected_pte; > > + ignore_exclusive_pte =3D > > pte_swp_clear_exclusive(ignore_exclusive_pte); > > + ignore_exclusive_expected_pte =3D > > pte_swp_clear_exclusive(expected_pte); > > + > > + if (!pte_same(ignore_exclusive_pte, > > ignore_exclusive_expected_pte)) > > + break; > > + } else { > > + if (!pte_same(pte, expected_pte)) > > + break; > > + } > > > > if (any_shared) > > *any_shared |=3D !pte_swp_exclusive(pte); > > I'll leave David to comment on this proposal; I'm not sure I understand a= ll the > details. The code change does look a bit "busy" though - sometimes that c= an be > an indicator :) indeed. I wrote it in one minute. I'm confident that the code can be written in a manner similar to __pte_batch_clear_ignored. I was only proposing the approach, not selling the code :-) > > > > >> softdirty and uffd-wp as well at least initially for simplicity. > > > > yes for this. > > > > By the way, I wonder if you and Ryan have a moment to review swpin > > refault patchset > > v2 :-) > > It's on my todo list! I'm very keen to get as much large swap-out and swa= p-in > support into v6.10 as we can. Hoping to get to it inthe next couple of da= ys. > > > > > [PATCH v2 0/5] large folios swap-in: handle refault cases first > > https://lore.kernel.org/linux-mm/20240409082631.187483-1-21cnbao@gmail.= com/ > > > > > >> > >> -- > >> Cheers, > >> > >> David / dhildenb > >> > > Thanks Barry >