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;
IronPort-SDR: M5EIEsAo9IUlYXQ9fj8IK7R9TAjjf751f6Mab25GQ/A/4W07lmjbtoxhxfgtnSt5EcKjzCgtOi
 Z+Ygmz8ATj4g==
IronPort-SDR: 4xXBe1wnECRz/UV85LMKUwzgWH5fHFhfCBvqCT4xRlF3F0nTHNQLb9gHFmsHaGegX0rnJCKXYe
 +nq561+7bwVg==
From:   "Huang\, Ying" <ying.huang@intel.com>
To:     Andrea Righi <andrea.righi@canonical.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Minchan Kim <minchan@kernel.org>,
        Anchal Agarwal <anchalag@amazon.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] mm: swap: properly update readahead statistics in unuse_pte_range()
References: <20200416180132.GB3352@xps-13>
Date:   Fri, 17 Apr 2020 11:01:22 +0800
In-Reply-To: <20200416180132.GB3352@xps-13> (Andrea Righi's message of "Thu,
        16 Apr 2020 20:01:32 +0200")
Message-ID: <871romvmrh.fsf@yhuang-dev.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Andrea Righi <andrea.righi@canonical.com> writes:

> In unuse_pte_range() we blindly swap-in pages without checking if the
> swap entry is already present in the swap cache.
>
> By doing this, the hit/miss ratio used by the swap readahead heuristic
> is not properly updated and this leads to non-optimal performance during
> swapoff.

It's more important to describe why we need this patch in the patch
description.  So, please add some information about your use case.  And
please focus on the technical part instead of the business part.

> Tracing the distribution of the readahead size returned by the swap
> readahead heuristic during swapoff shows that a small readahead size is
> used most of the time as if we had only misses (this happens both with
> cluster and vma readahead), for example:
>
> r::swapin_nr_pages(unsigned long offset):unsigned long:$retval
>         COUNT      EVENT
>         36948      $retval = 8
>         44151      $retval = 4
>         49290      $retval = 1
>         527771     $retval = 2
>
> Checking if the swap entry is present in the swap cache, instead, allows
> to properly update the readahead statistics and the heuristic behaves in
> a better way during swapoff, selecting a bigger readahead size:
>
> r::swapin_nr_pages(unsigned long offset):unsigned long:$retval
>         COUNT      EVENT
>         1618       $retval = 1
>         4960       $retval = 2
>         41315      $retval = 4
>         103521     $retval = 8
>
> In terms of swapoff performance the result is the following:
>
> Testing environment
> ===================
>
>  - Host:
>    CPU: 1.8GHz Intel Core i7-8565U (quad-core, 8MB cache)
>    HDD: PC401 NVMe SK hynix 512GB
>    MEM: 16GB
>
>  - Guest (kvm):
>    8GB of RAM
>    virtio block driver
>    16GB swap file on ext4 (/swapfile)
>
> Test case
> =========
>  - allocate 85% of memory
>  - `systemctl hibernate` to force all the pages to be swapped-out to the
>    swap file
>  - resume the system
>  - measure the time that swapoff takes to complete:
>    # /usr/bin/time swapoff /swapfile
>
> Result (swapoff time)
> ======
>                   5.6 vanilla   5.6 w/ this patch
>                   -----------   -----------------
> cluster-readahead      22.09s              12.19s
>     vma-readahead      18.20s              15.33s
>
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>

Thanks!  But you don't need to do this.  You can add my Reviewed-by after
we have finished the work on patch description.

Best Regards,
Huang, Ying