| 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
| ||
|
Message-ID: <CAMgjq7AoJ+m64e2rWFFjU943D8kCZoR_e+Hd8LnT3bAy=gTT_w@mail.gmail.com>
Date: Thu, 19 Jun 2025 18:50:37 +0800
From: Kairui Song <ryncsn@...il.com>
To: Baoquan He <bhe@...hat.com>
Cc: linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
Matthew Wilcox <willy@...radead.org>, Hugh Dickins <hughd@...gle.com>, Chris Li <chrisl@...nel.org>,
David Hildenbrand <david@...hat.com>, Yosry Ahmed <yosryahmed@...gle.com>,
"Huang, Ying" <ying.huang@...ux.alibaba.com>, Nhat Pham <nphamcs@...il.com>,
Johannes Weiner <hannes@...xchg.org>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
Barry Song <baohua@...nel.org>, Kalesh Singh <kaleshsingh@...gle.com>,
Kemeng Shi <shikemeng@...weicloud.com>, Tim Chen <tim.c.chen@...ux.intel.com>,
Ryan Roberts <ryan.roberts@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 20/28] mm, swap: check swap table directly for checking cache
On Thu, Jun 19, 2025 at 6:38 PM Baoquan He <bhe@...hat.com> wrote:
>
> On 05/15/25 at 04:17am, Kairui Song wrote:
> > From: Kairui Song <kasong@...cent.com>
> >
> > Instead of looking at the swap map, check swap table directly to tell if
> > a swap entry has cache. Prepare for remove SWAP_HAS_CACHE.
>
> But you actually check both the swap table entry and swap map entry in
> this patch, or do I miss anything?
Hi, Baoquan
>
> E.g
>
> if (!swap_count(si->swap_map[offset]) && swp_te_is_folio(swp_te))
Yes, the count info is still in the swap_map now, I'm only converting
the HAS_CACHE check to use swp_te_t here. We'll remove swap_map in
later patches and use the swp_te_t solely to get both info.
The reason some checks are added to check the swap_count is that:
Before this patch, `swap_map[offset] == SWAP_HAS_CACHE` implies the
count is zero too. So if HAS_CACHE is moved to swp_te_t, we still need
to check the count separately. The overhead will be gone very soon in
a later patch.
>
> >
> > Signed-off-by: Kairui Song <kasong@...cent.com>
> > ---
> > mm/memory.c | 12 +++++------
> > mm/swap.h | 6 ++++++
> > mm/swap_state.c | 11 ++++++++++
> > mm/swapfile.c | 54 +++++++++++++++++++++++--------------------------
> > 4 files changed, 48 insertions(+), 35 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index a70624a55aa2..a9a548575e72 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4314,15 +4314,15 @@ static struct folio *__alloc_swap_folio(struct vm_fault *vmf)
> > }
> >
> > #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> > -static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
> > +static inline int non_swapcache_batch(swp_entry_t entry, unsigned int max_nr)
> > {
> > - struct swap_info_struct *si = swp_info(entry);
> > - pgoff_t offset = swp_offset(entry);
> > - int i;
> > + unsigned int i;
> >
> > for (i = 0; i < max_nr; i++) {
> > - if ((si->swap_map[offset + i] & SWAP_HAS_CACHE))
> > - return i;
> > + /* Page table lock pins the swap entries / swap device */
> > + if (swap_cache_check_folio(entry))
> > + break;
> > + entry.val++;
> > }
> >
> > return i;
> > diff --git a/mm/swap.h b/mm/swap.h
> > index 467996dafbae..2ae4624a0e48 100644
> > --- a/mm/swap.h
> > +++ b/mm/swap.h
> > @@ -186,6 +186,7 @@ static inline struct address_space *swap_address_space(swp_entry_t entry)
> > extern struct folio *swap_cache_get_folio(swp_entry_t entry);
> > extern struct folio *swap_cache_add_folio(swp_entry_t entry, struct folio *folio,
> > void **shadow, bool swapin);
> > +extern bool swap_cache_check_folio(swp_entry_t entry);
> > extern void *swap_cache_get_shadow(swp_entry_t entry);
> > /* Below helpers requires the caller to lock the swap cluster. */
> > extern void __swap_cache_del_folio(swp_entry_t entry,
> > @@ -395,6 +396,11 @@ static inline void *swap_cache_get_shadow(swp_entry_t end)
> > return NULL;
> > }
> >
> > +static inline bool swap_cache_check_folio(swp_entry_t entry)
> > +{
> > + return false;
> > +}
> > +
> > static inline unsigned int folio_swap_flags(struct folio *folio)
> > {
> > return 0;
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index c8bb16835612..ea6a1741db5c 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -266,6 +266,17 @@ struct folio *swap_cache_get_folio(swp_entry_t entry)
> > return folio;
> > }
> >
> > +/*
> > + * Check if a swap entry has folio cached, may return false positive.
> > + * Caller must hold a reference of the swap device or pin it in other ways.
> > + */
> > +bool swap_cache_check_folio(swp_entry_t entry)
> > +{
> > + swp_te_t swp_te;
> > + swp_te = __swap_table_get(swp_cluster(entry), swp_offset(entry));
> > + return swp_te_is_folio(swp_te);
> > +}
> > +
> > /*
> > * If we are the only user, then try to free up the swap cache.
> > *
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index ef233466725e..0f2a499ff2c9 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -181,15 +181,19 @@ static long swap_usage_in_pages(struct swap_info_struct *si)
> > #define TTRS_FULL 0x4
> >
> > static bool swap_only_has_cache(struct swap_info_struct *si,
> > - unsigned long offset, int nr_pages)
> > + struct swap_cluster_info *ci,
> > + unsigned long offset, int nr_pages)
> > {
> > unsigned char *map = si->swap_map + offset;
> > unsigned char *map_end = map + nr_pages;
> > + swp_te_t entry;
> >
> > do {
> > + entry = __swap_table_get(ci, offset);
>
> entry is not used in swap_only_has_cache() in this patch.
Thanks, it used in a later patch so I must move it here accidently
during a rebase, will defer this change to later patch.
>
> > VM_BUG_ON(!(*map & SWAP_HAS_CACHE));
> > - if (*map != SWAP_HAS_CACHE)
> > + if (*map)
> > return false;
> > + offset++;
> > } while (++map < map_end);
> >
> > return true;
> ......snip...
>
>
Powered by blists - more mailing lists