| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Jan 2024 14:29:00 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org, Kairui Song <kasong@...cent.com>, Andrew Morton
<akpm@...ux-foundation.org>, Chris Li <chrisl@...nel.org>, Hugh Dickins
<hughd@...gle.com>, Johannes Weiner <hannes@...xchg.org>, Matthew Wilcox
<willy@...radead.org>, Michal Hocko <mhocko@...e.com>, Yosry Ahmed
<yosryahmed@...gle.com>, David Hildenbrand <david@...hat.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/7] mm/swap: introduce swapin_entry for unified
readahead policy
Kairui Song <ryncsn@...il.com> writes:
> From: Kairui Song <kasong@...cent.com>
>
> Introduce swapin_entry which merges swapin_readahead and swapin_direct
> making it the main entry for swapin pages, and use a unified swapin
> readahead policy.
>
> This commit makes swapoff make use of this new helper and skip readahead
> for SYNCHRONOUS_IO device since it's not helpful here. Now swapping
> off a 10G ZRAM (lzo-rle) after same workload is faster since readahead
> is skipped and overhead is reduced.
>
> Before:
> time swapoff /dev/zram0
> real 0m12.337s
> user 0m0.001s
> sys 0m12.329s
>
> After:
> time swapoff /dev/zram0
> real 0m9.728s
> user 0m0.001s
> sys 0m9.719s
>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> ---
> mm/memory.c | 18 +++---------------
> mm/swap.h | 16 ++++------------
> mm/swap_state.c | 40 ++++++++++++++++++++++++----------------
> mm/swapfile.c | 7 ++-----
> 4 files changed, 33 insertions(+), 48 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 81dc9d467f4e..8711f8a07039 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3864,20 +3864,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> swapcache = folio;
>
> if (!folio) {
> - if (data_race(si->flags & SWP_SYNCHRONOUS_IO) &&
> - __swap_count(entry) == 1) {
> - /* skip swapcache and readahead */
> - folio = swapin_direct(entry, GFP_HIGHUSER_MOVABLE, vmf);
> - if (folio)
> - page = &folio->page;
> - } else {
> - page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> - vmf);
> - if (page)
> - folio = page_folio(page);
> - swapcache = folio;
> - }
> -
> + folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
> + vmf, &swapcache);
> if (!folio) {
> /*
> * Back out if somebody else faulted in this pte
> @@ -3890,11 +3878,11 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> ret = VM_FAULT_OOM;
> goto unlock;
> }
> -
Change by accident?
> /* Had to read the page from swap area: Major fault */
> ret = VM_FAULT_MAJOR;
> count_vm_event(PGMAJFAULT);
> count_memcg_event_mm(vma->vm_mm, PGMAJFAULT);
> + page = folio_file_page(folio, swp_offset(entry));
Better to move this line just after checking !folio. This make it a
little easier to associate page to folio.
> } else if (PageHWPoison(page)) {
> /*
> * hwpoisoned dirty swapcache pages are kept for killing
> diff --git a/mm/swap.h b/mm/swap.h
> index 83eab7b67e77..8f8185d3865c 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -54,10 +54,8 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_flags,
> bool skip_if_exists);
> struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
> struct mempolicy *mpol, pgoff_t ilx);
> -struct page *swapin_readahead(swp_entry_t entry, gfp_t flag,
> - struct vm_fault *vmf);
> -struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
> - struct vm_fault *vmf);
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t flag,
> + struct vm_fault *vmf, struct folio **swapcached);
>
> static inline unsigned int folio_swap_flags(struct folio *folio)
> {
> @@ -88,14 +86,8 @@ static inline struct folio *swap_cluster_readahead(swp_entry_t entry,
> return NULL;
> }
>
> -struct folio *swapin_direct(swp_entry_t entry, gfp_t flag,
> - struct vm_fault *vmf)
> -{
> - return NULL;
> -}
> -
> -static inline struct page *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
> - struct vm_fault *vmf)
> +static inline struct folio *swapin_entry(swp_entry_t swp, gfp_t gfp_mask,
> + struct vm_fault *vmf, struct folio **swapcached)
> {
> return NULL;
> }
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index a450d09fc0db..5e06b2e140d4 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -870,8 +870,8 @@ static struct folio *swap_vma_readahead(swp_entry_t targ_entry, gfp_t gfp_mask,
> * Returns the struct folio for entry and addr after the swap entry is read
> * in.
> */
> -struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
> - struct vm_fault *vmf)
> +static struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
> + struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> struct folio *folio;
> @@ -908,33 +908,41 @@ struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask,
> }
>
> /**
> - * swapin_readahead - swap in pages in hope we need them soon
> + * swapin_entry - swap in a folio from swap entry
> * @entry: swap entry of this memory
> * @gfp_mask: memory allocation flags
> * @vmf: fault information
> + * @swapcache: set to the swapcache folio if swapcache is used
> *
> * Returns the struct page for entry and addr, after queueing swapin.
> *
> - * It's a main entry function for swap readahead. By the configuration,
> + * It's the main entry function for swap in. By the configuration,
> * it will read ahead blocks by cluster-based(ie, physical disk based)
> - * or vma-based(ie, virtual address based on faulty address) readahead.
> + * or vma-based(ie, virtual address based on faulty address) readahead,
> + * or skip the readahead(ie, ramdisk based swap device).
> */
> -struct page *swapin_readahead(swp_entry_t entry, gfp_t gfp_mask,
> - struct vm_fault *vmf)
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp_mask,
> + struct vm_fault *vmf, struct folio **swapcache)
> {
> struct mempolicy *mpol;
> - pgoff_t ilx;
> struct folio *folio;
> + pgoff_t ilx;
ditto.
Otherwise, looks good to me. Thanks!
Reviewed-by: "Huang, Ying" <ying.huang@...el.com>
>
> - mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> - folio = swap_use_vma_readahead() ?
> - swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf) :
> - swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> - mpol_cond_put(mpol);
> + if (data_race(swp_swap_info(entry)->flags & SWP_SYNCHRONOUS_IO) &&
> + __swap_count(entry) == 1) {
> + folio = swapin_direct(entry, gfp_mask, vmf);
> + } else {
> + mpol = get_vma_policy(vmf->vma, vmf->address, 0, &ilx);
> + if (swap_use_vma_readahead())
> + folio = swap_vma_readahead(entry, gfp_mask, mpol, ilx, vmf);
> + else
> + folio = swap_cluster_readahead(entry, gfp_mask, mpol, ilx);
> + mpol_cond_put(mpol);
> + if (swapcache)
> + *swapcache = folio;
> + }
>
> - if (!folio)
> - return NULL;
> - return folio_file_page(folio, swp_offset(entry));
> + return folio;
> }
>
> #ifdef CONFIG_SYSFS
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 606d95b56304..1cf7e72e19e3 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1867,7 +1867,6 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
>
> folio = swap_cache_get_folio(entry, vma, addr);
> if (!folio) {
> - struct page *page;
> struct vm_fault vmf = {
> .vma = vma,
> .address = addr,
> @@ -1875,10 +1874,8 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> .pmd = pmd,
> };
>
> - page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
> - &vmf);
> - if (page)
> - folio = page_folio(page);
> + folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
> + &vmf, NULL);
> }
> if (!folio) {
> /*
Powered by blists - more mailing lists