Re: [PATCH 2/2] mm,swap: skip swap readahead if page was obtained instantaneously

[huang ying <huang.ying.caritas@gmail.com>]

On Tue, Sep 22, 2020 at 10:02 AM Rik van Riel <riel@surriel.com> wrote:
>
> Check whether a swap page was obtained instantaneously, for example
> because it is in zswap, or on a very fast IO device which uses busy
> waiting, and we did not wait on IO to swap in this page.
> If no IO was needed to get the swap page we want, kicking off readahead
> on surrounding swap pages is likely to be counterproductive, because the
> extra loads will cause additional latency, use up extra memory, and chances
> are the surrounding pages in swap are just as fast to load as this one,
> making readahead pointless.
>
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  mm/swap_state.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index aacb9ba53f63..6919f9d5fe88 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -637,6 +637,7 @@ static struct page *swap_cluster_read_one(swp_entry_t entry,
>  struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>                                 struct vm_fault *vmf)

Why not do this for swap_vma_readahead() too?  swap_cluster_read_one()
can be used in swap_vma_readahead() too.

>  {
> +       struct page *page;
>         unsigned long entry_offset = swp_offset(entry);
>         unsigned long offset = entry_offset;
>         unsigned long start_offset, end_offset;
> @@ -668,11 +669,18 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>                 end_offset = si->max - 1;
>
>         blk_start_plug(&plug);
> +       /* If we read the page without waiting on IO, skip readahead. */
> +       page = swap_cluster_read_one(entry, offset, gfp_mask, vma, addr, false);
> +       if (page && PageUptodate(page))
> +               goto skip_unplug;
> +
> +       /* Ok, do the async read-ahead now. */
>         for (offset = start_offset; offset <= end_offset ; offset++) {
> -               /* Ok, do the async read-ahead now */
> -               swap_cluster_read_one(entry, offset, gfp_mask, vma, addr,
> -                                     offset != entry_offset);
> +               if (offset == entry_offset)
> +                       continue;
> +               swap_cluster_read_one(entry, offset, gfp_mask, vma, addr, true);
>         }
> +skip_unplug:
>         blk_finish_plug(&plug);
>
>         lru_add_drain();        /* Push any new pages onto the LRU now */

Best Regards,
Huang, Ying