Re: [PATCH v2 1/2] mm: swap: fix performance regression on sparsetruncate-tiny

[Mel Gorman]

On Thu, Apr 06, 2023 at 12:18:53AM +0800, Qi Zheng wrote:
> The ->percpu_pvec_drained was originally introduced by
> commit d9ed0d08b6c6 ("mm: only drain per-cpu pagevecs once per
> pagevec usage") to drain per-cpu pagevecs only once per pagevec
> usage. But after converting the swap code to be more folio-based,
> the commit c2bc16817aa0 ("mm/swap: add folio_batch_move_lru()")
> breaks this logic, which would cause ->percpu_pvec_drained to be
> reset to false, that means per-cpu pagevecs will be drained
> multiple times per pagevec usage.
> 
> In theory, there should be no functional changes when converting
> code to be more folio-based. We should call folio_batch_reinit()
> in folio_batch_move_lru() instead of folio_batch_init(). And to
> verify that we still need ->percpu_pvec_drained, I ran
> mmtests/sparsetruncate-tiny and got the following data:
> 
>                              baseline                   with
>                             baseline/                 patch/
> Min       Time      326.00 (   0.00%)      328.00 (  -0.61%)
> 1st-qrtle Time      334.00 (   0.00%)      336.00 (  -0.60%)
> 2nd-qrtle Time      338.00 (   0.00%)      341.00 (  -0.89%)
> 3rd-qrtle Time      343.00 (   0.00%)      347.00 (  -1.17%)
> Max-1     Time      326.00 (   0.00%)      328.00 (  -0.61%)
> Max-5     Time      327.00 (   0.00%)      330.00 (  -0.92%)
> Max-10    Time      328.00 (   0.00%)      331.00 (  -0.91%)
> Max-90    Time      350.00 (   0.00%)      357.00 (  -2.00%)
> Max-95    Time      395.00 (   0.00%)      390.00 (   1.27%)
> Max-99    Time      508.00 (   0.00%)      434.00 (  14.57%)
> Max       Time      547.00 (   0.00%)      476.00 (  12.98%)
> Amean     Time      344.61 (   0.00%)      345.56 *  -0.28%*
> Stddev    Time       30.34 (   0.00%)       19.51 (  35.69%)
> CoeffVar  Time        8.81 (   0.00%)        5.65 (  35.87%)
> BAmean-99 Time      342.38 (   0.00%)      344.27 (  -0.55%)
> BAmean-95 Time      338.58 (   0.00%)      341.87 (  -0.97%)
> BAmean-90 Time      336.89 (   0.00%)      340.26 (  -1.00%)
> BAmean-75 Time      335.18 (   0.00%)      338.40 (  -0.96%)
> BAmean-50 Time      332.54 (   0.00%)      335.42 (  -0.87%)
> BAmean-25 Time      329.30 (   0.00%)      332.00 (  -0.82%)
> 
> From the above it can be seen that we get similar data to when
> ->percpu_pvec_drained was introduced, so we still need it. Let's
> call folio_batch_reinit() in folio_batch_move_lru() to restore
> the original logic.
> 
> Fixes: c2bc16817aa0 ("mm/swap: add folio_batch_move_lru()")
> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>

Well spotted,

Acked-by: Mel Gorman <mgorman@xxxxxxx>

-- 
Mel Gorman
SUSE Labs