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 08885C67861 for ; Tue, 9 Apr 2024 09:41:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9226B6B0093; Tue, 9 Apr 2024 05:41:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D2DA6B00A0; Tue, 9 Apr 2024 05:41:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 773FF6B00A3; Tue, 9 Apr 2024 05:41:53 -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 589356B0093 for ; Tue, 9 Apr 2024 05:41:53 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 1368AA033D for ; Tue, 9 Apr 2024 09:41:53 +0000 (UTC) X-FDA: 81989501706.01.F41C06C Received: from mail-vk1-f177.google.com (mail-vk1-f177.google.com [209.85.221.177]) by imf27.hostedemail.com (Postfix) with ESMTP id 396384000C for ; Tue, 9 Apr 2024 09:41:51 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="OLM/OPZV"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf27.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.177 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712655711; a=rsa-sha256; cv=none; b=4xRj7GZutO3034IIic4jPE9tw3JnMlWoXQ3I70Njc8MzdJ1kdBUQqDvakoDNsfzItsrtjy 0iFDjyu5idupTkdAXveOGM5eyTFJDnQwOjmANbfp45rQyGlmd6b4r3ofWAd/Mi8Hfh2bis oHOwU/Gw+CYbnsxkdJWLDoGyYiSkz04= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b="OLM/OPZV"; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf27.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.221.177 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=1712655711; 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=e75nlwa4RFxqNbzR1xYnRqzE3pogwi/N6HklJl3ZIUs=; b=kK7Fu1kCjHjzXmSrJ1KPXvve2HJ+H7YJ16kcOslxsJuXoZubshFIlIe7Ip0/HM24lesiu8 TYPnRnsHW4lGzypx3k90aXu8Dnc7XELbIhCpyBiG0KzCGGDvs6dpUwD9SDRB7tJjNpudx1 VCdhaVb0gfSCkAO9+Cgq5VCgNFWMh14= Received: by mail-vk1-f177.google.com with SMTP id 71dfb90a1353d-4daa2b076beso1591162e0c.1 for ; Tue, 09 Apr 2024 02:41:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712655710; x=1713260510; 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=e75nlwa4RFxqNbzR1xYnRqzE3pogwi/N6HklJl3ZIUs=; b=OLM/OPZVmh8wmzSM3OBcf8DmT7RsieQAnpC90dYXLOMV1lDtlCmuF8VkTWCkpkQZvR xK9SiUN1KU6rVSkB8wQdcInHH/WzPGa35SjGun2hNbGaI3x5QwDfz52OFcwReqNHXIEg Ro00JTh13Ttm/l1eF/rZblTLk5FvSC+CkHRuVx6CkSIbcM/5KTq6CptTDI8nMVr3tuvg UcJ7GdGCvPJiS7i2hp9qXhC+RQw+6ku4X2Fbt7Mq6hJ5udOup5EQaW/BngyOV7IRkVre ZU8LSRYJcAw1UX5siNln5ew0tA92UwYVUfQkEE90o+NiGBlCXoPc2Nc6u88SvCfBFXF6 m60g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712655710; x=1713260510; 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=e75nlwa4RFxqNbzR1xYnRqzE3pogwi/N6HklJl3ZIUs=; b=oW40NrDuXH6Ijd1ZS2wsKLd7jvG0mn6APfrsSxlAEQ3A3JPHm0eY/gcR69jqEyDB+7 0IKq4TM/Q9BLaWrxLLkKrl90EJKFPeeNeCM3sfUzZo9uWAD/jiC0kgC3cRtsX3DV/DgX HZXqjPc3O53YrcADZanDmx6rC389x1J1p52ZOT/TthZFb8abbT16zXX/LJDecqwc973v p13EkWlaj2iN79YrquL/vEQkBS1HHPTvgg6V02fm/C0nfyFNewlisyOVwe9c6Y6qMgPt 6FN11hvo+nSYHo7oTVuWdQOQqPlqQzDD2aPpMwNNBzS2GikIvd6VnHUiOd1yudIFEp9t rt4A== X-Forwarded-Encrypted: i=1; AJvYcCUMLJHr8M2o+DY/RzwtZBJ6ZmOvfe58nwhDrzd5kKw7RuFueNJ9hInMyM/IsbhSvMUp4sJTdzBxRnxtnpFFd9AnfOo= X-Gm-Message-State: AOJu0YzPSccZAX08essQ78gD8BMdQ/0kAhMDxn86DR2OY4NAo79TABuF zwzge2GRPn5XwZlJxmnn51HgjH2ebOJzZLkUl/HcCDVJAKBZLmL10IRQTNXzdQPhE5uSpmkAhvs Zz/p1KdcVskvmTJHXXNlA4HPP6Z8= X-Google-Smtp-Source: AGHT+IFKrU4IbiGWPR31h+jDsMjUbsv3xibBiJoMps6UGegTLCB20cs+Kh3Bcr+daKDwtGZvG6CSw1xTycpi5rsz+dE= X-Received: by 2002:a05:6122:1d05:b0:4d4:21cc:5f4f with SMTP id gc5-20020a0561221d0500b004d421cc5f4fmr7129356vkb.11.1712655710067; Tue, 09 Apr 2024 02:41:50 -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: <44e23a4d-97e7-4d84-848a-f9325b7b76c0@redhat.com> From: Barry Song <21cnbao@gmail.com> Date: Tue, 9 Apr 2024 21:41:39 +1200 Message-ID: Subject: Re: [PATCH v7 2/7] mm: swap: free_swap_and_cache_nr() as batched free_swap_and_cache() To: David Hildenbrand Cc: Ryan Roberts , 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-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 396384000C X-Stat-Signature: qowkbzpn55wunsj74sofrx4eddmqandf X-HE-Tag: 1712655711-747195 X-HE-Meta: U2FsdGVkX19icAUd2iKJQxUiY0XvjiYHFGIrqxqHhkINVUO7uqi9aKXlHLN3/DdK98xlX77SjIkbT9OvszOIQdyiZY/GwvsC6WjD8H5OOurgPBzHwO2OxRoVE6UHS/Y5FrR3n/bdfW0/2JzVoa5yxugElC0/qwywmyj8m3iFeQz6sdmYttjEQz+Av6Ebze70OdJKyno2N6Pfq2cMjSyLg56EcvF1zrZFCjV/IQLTt0JNyaZXYd6Uy4Uq2PFvBvuaNRd7PoG4N6LJuG73uZQa9/Tin6DWEQGP0wmBZQZofkOgy1C9epjMr3PX/WHvanpp6+fIglAsodfHacv9rwQv9G2F3sSqrzHEI/Qf1ZqptgJP03Um91Of9s8vbx28PHKfdFI6Mf9LP5a5EIqr1WfF+1PWwt84QxKy4fDBpl7y4qz+f9gRSvPzScnT3hvtwGxRfukm4T6Ce19DlJinFNbOKfuE077XhIZbbBjFYHqlPJaWEcGlmV5Qw508Is94KrI9P4gCPeMpDj+j6WW0UHDm7eAJSUIhBu4JCk5Yimfwsesh24cBZ0tPPRgisl7odxEhGQjWen8qK4Xe6AsHNPqc1Gl+XJ1WWuZe0+Q55FX6EX/FAlhdoDjvFIEyCTFXcYqk+NhscEvT4M0bh2dJARr6FGkqsHCdSsmY/sb/fYgD4VkSDsMRgWJiydXMYhfxq4QlrjWp+ee5xhD6a+vCqo7ozdESpZZxWoXXEiTJMA4Eu4TyoSil8XkACmYyEog3tZ7sAaGZS9KriIxb+n9h44ePw7WoRTlvztBJqi/O6voTuVCD9+EpUHQDLmX6UB06fszrgAqBJLAvcGV2lOLwblI4Wxx3PXymk2NWoy5tIDREcDv/+Pe2v6fXNhymOOfrtFpunUfX+rswP1S6+yhOcJ3FC2IOzS/GtxBchJpQuSXA6IDs1NwNdsVpORwbdMpwV1Phiv3Xi0mJvzLqDLjpm9a QfGFWwxP nYZmAqTzwf5Qd+v1PDM1wERMXL6LmlS/v0m6QzIF+Xos47HDOo2Qv3yWnQakWGQGegAR7vNiQDgDs3WUnCjpJRXpy1uL34720qlt2o32Y3T898VcnWqmwkvNDdj3EamPdpYQkApnrG3v3cWoxSQi/Pu+TroTNBPpPCSo1Nrs54jdL3OYtneaX7Lipb2mvEgoeX6mpHkKZnvEn5yWehzeN8QCHmwnC1oPpKqkp/XvWIPkTzMVXG/Khkjz9i+3XRWyYSm/AxAR2WDpaaQcix6K7G/RayAYcALthU7Llq509m3Fg3C2mgUtas3TOn+Nc0xHZyehyH8diu4m/oPhh0DGhOiok+HIJQ46OhrG446MxZmAyMZKIZ0o9mrLc8PmF9QHK20ATlc1A8GKAHrU6hyJ1bc4t9+uXdSsnFWDMJkLIDz1RCo33r7eYRhUcEo7aGDmxxejESO/4T73YJX0dt9umzhyfPTxaEI2l2T9X3YMhdH3yPZcO4FmPPjKli3yU3Cg9LPQvosd33ZTJGK/g2E82hRdJr8pCj0fJagDaU/Gr9KX3Da6I0E9QZwYR9H/e9U46/Iol 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: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> w= rote: > >> > >> 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 determ= ine > >>> if a folio is large, free_swap_and_cache() will take a reference and > >>> lock a large folio much more often, which could lead to contention an= d > >>> (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 swa= p > >>> entries together. This allows us to first drop a reference to each sw= ap > >>> slot before we try to release the cache folio. This means we only try= to > >>> release the folio once, only taking the reference and lock once - muc= h > >>> 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 b= oth > >>> 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 wi= th > >>> most of the information from get_swap_device(). With the batch versio= n, > >>> we wouldn't know which pte was bad anyway so could print the wrong on= e. > >>> > >>> 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(st= ruct mm_struct *mm, > >>> } > >>> #endif > >>> > >>> +#ifndef clear_not_present_full_ptes > >>> +/** > >>> + * clear_not_present_full_ptes - Clear multiple not present PTEs whi= ch 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 as = a simple > >>> + * loop over pte_clear_not_present_full(). > >>> + * > >>> + * Context: The caller holds the page table lock. The PTEs are all = not present. > >>> + * The PTEs are all in the same PMD. > >>> + */ > >>> +static inline void clear_not_present_full_ptes(struct mm_struct *mm, > >>> + unsigned long addr, pte_t *ptep, unsigned int nr, int= 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_in= fo_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 nr) > >>> +{ > >>> +} > >>> > >>> static inline void free_swap_cache(struct folio *folio) > >>> { > >>> @@ -589,6 +590,11 @@ static inline int add_swap_extent(struct swap_in= fo_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, i= ncluding > >>> + * 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 think = I should > > entirely drop: > > > > [PATCH v2 3/5] mm: swap_pte_batch: add an output argument to reture if > > all swap entries are exclusive > > Yes. If we ever want to ignore some bits, we should likely add flags to > 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 either drop my 3/5 or ignore the exclusive bit as below (if anyone is not shared, swpin won't reuse the large folio, but it can still entirely map it read-on= ly): 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); > 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 :-) [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