On 02/15/25 at 01:57am, Kairui Song wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> There are only two callers of __read_swap_cache_async not holding a swap
> device reference, so make them hold a reference instead, and drop the
> get/put_swap_device calls in __read_swap_cache_async. This should reduce
> the overhead for swap in during page fault slightly.
This looks good to me, while the log makes me take a little longer time
to understand. Maybe rephrasing them a little bit can facilitate the log
reading. Not sure if my understanding is correct.
=======
Currently, __read_swap_cache_async() has get/put_swap_device() calls to
increase/decrease a swap device reference. While some of its callers
have held the swap device reference, e.g in do_swap_page() and
shmem_swapin_folio() where __read_swap_cache_async() will finally
called. Now there are only two excpetional callers not holding a swap
device reference, so make them hold a reference instead. And drop the
get/put_swap_device calls in __read_swap_cache_async. This should reduce
the overhead for swap in during page fault slightly.
========
Other than the nit in log, this looks good to me.
Reviewed-by: Baoquan He <bhe@xxxxxxxxxx>
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
> mm/swap_state.c | 14 ++++++++------
> mm/zswap.c | 6 ++++++
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index a54b035d6a6c..50840a2887a5 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -426,17 +426,13 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
> struct mempolicy *mpol, pgoff_t ilx, bool *new_page_allocated,
> bool skip_if_exists)
> {
> - struct swap_info_struct *si;
> + struct swap_info_struct *si = swp_swap_info(entry);
> struct folio *folio;
> struct folio *new_folio = NULL;
> struct folio *result = NULL;
> void *shadow = NULL;
>
> *new_page_allocated = false;
> - si = get_swap_device(entry);
> - if (!si)
> - return NULL;
> -
> for (;;) {
> int err;
> /*
> @@ -532,7 +528,6 @@ struct folio *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
> put_swap_folio(new_folio, entry);
> folio_unlock(new_folio);
> put_and_return:
> - put_swap_device(si);
> if (!(*new_page_allocated) && new_folio)
> folio_put(new_folio);
> return result;
> @@ -552,11 +547,16 @@ struct folio *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
> struct vm_area_struct *vma, unsigned long addr,
> struct swap_iocb **plug)
> {
> + struct swap_info_struct *si;
> bool page_allocated;
> struct mempolicy *mpol;
> pgoff_t ilx;
> struct folio *folio;
>
> + si = get_swap_device(entry);
> + if (!si)
> + return NULL;
> +
> mpol = get_vma_policy(vma, addr, 0, &ilx);
> folio = __read_swap_cache_async(entry, gfp_mask, mpol, ilx,
> &page_allocated, false);
> @@ -564,6 +564,8 @@ struct folio *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>
> if (page_allocated)
> swap_read_folio(folio, plug);
> +
> + put_swap_device(si);
> return folio;
> }
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index ac9d299e7d0c..83dfa1f9e689 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1051,14 +1051,20 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> struct folio *folio;
> struct mempolicy *mpol;
> bool folio_was_allocated;
> + struct swap_info_struct *si;
> struct writeback_control wbc = {
> .sync_mode = WB_SYNC_NONE,
> };
>
> /* try to allocate swap cache folio */
> + si = get_swap_device(swpentry);
> + if (!si)
> + return -EEXIST;
> +
> mpol = get_task_policy(current);
> folio = __read_swap_cache_async(swpentry, GFP_KERNEL, mpol,
> NO_INTERLEAVE_INDEX, &folio_was_allocated, true);
> + put_swap_device(si);
> if (!folio)
> return -ENOMEM;
>
> --
> 2.48.1
>
