Re: [PATCH 08/24] mm/swap: check readahead policy per entry

[Chris Li]

On Sun, Nov 19, 2023 at 11:48 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Currently VMA readahead is globally disabled when any rotate disk is
> used as swap backend. So multiple swap devices are enabled, if a slower
> hard disk is set as a low priority fallback, and a high performance SSD
> is used and high priority swap device, vma readahead is disabled globally.
> The SSD swap device performance will drop by a lot.
>
> Check readahead policy per entry to avoid such problem.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/swap_state.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index ff6756f2e8e4..fb78f7f18ed7 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -321,9 +321,9 @@ static inline bool swap_use_no_readahead(struct swap_info_struct *si, swp_entry_
>         return data_race(si->flags & SWP_SYNCHRONOUS_IO) && __swap_count(entry) == 1;
>  }
>
> -static inline bool swap_use_vma_readahead(void)
> +static inline bool swap_use_vma_readahead(struct swap_info_struct *si)
>  {
> -       return READ_ONCE(enable_vma_readahead) && !atomic_read(&nr_rotate_swap);
> +       return data_race(si->flags & SWP_SOLIDSTATE) && READ_ONCE(enable_vma_readahead);

A very minor point:
I notice you change the order enable_vma_readahead to the last.
Normally if enable_vma_reachahead == 0, there is no need to check the si->flags.
The si->flags check is more expensive than simple memory load.
You might want to check enable_vma_readahead first then you can short
cut the more expensive part.

Chris