Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200922020148.3261797-1-riel@surriel.com> <20200922020148.3261797-3-riel@surriel.com>
In-Reply-To: <20200922020148.3261797-3-riel@surriel.com>
From:   huang ying <huang.ying.caritas@gmail.com>
Date:   Tue, 22 Sep 2020 11:13:12 +0800
Message-ID: <CAC=cRTP3Em2C7tRpbFyHztnjAvUMFn-9t1yqFCb=yc3MavpUcg@mail.gmail.com>
Subject: Re: [PATCH 2/2] mm,swap: skip swap readahead if page was obtained instantaneously
To:     Rik van Riel <riel@surriel.com>
Cc:     LKML <linux-kernel@vger.kernel.org>, linux-mm@kvack.org,
        kernel-team@fb.com, niketa@fb.com,
        Andrew Morton <akpm@linux-foundation.org>,
        Seth Jennings <sjenning@redhat.com>,
        Dan Streetman <ddstreet@ieee.org>,
        Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Huang Ying <ying.huang@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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