Re: [PATCH] mm: Remove stub for non_swap_entry()

[Alistair Popple]

Peter Xu <peterx@xxxxxxxxxx> writes:

> The stub for non_swap_entry() may not help much, because MAX_SWAPFILES has
> already contained all the information to decide whether a swap entry is
> real swap entry of pesudo ones (migrations, ...).
>
> There can be some performance influences on non_swap_entry() with below
> conditions all met:
>
>   !CONFIG_MIGRATION && !CONFIG_MEMORY_FAILURE && !CONFIG_DEVICE_PRIVATE
>
> But that's definitely not the major config most machines will use, at the
> meantime it's already in a slow path of swap entry (being parsed from a
> swap pte), so IMHO it shouldn't be a major issue.  Also according to the
> analysis from Alistair, somehow the stub didn't do the job right [1].

I wasn't so much concerned about execution speed given it's on the slow path
anyway but overall code size, which is one reason all those config options might
be disabled. However in practice it made little to no difference as those config
options already remove most of the extra code so I agree we can drop the stub.

Reviewed-by: Alistair Popple <apopple@xxxxxxxxxx>

> To make the code cleaner, let's drop the stub.
>
> [1] <https://lore.kernel.org/lkml/8735ihbw6g.fsf@nvdebian.thelocal/>
>
> Cc: Alistair Popple <apopple@xxxxxxxxxx>
> Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> ---
>
> Note: the uffd-wp shmem & hugetlbfs series will need this patch to make
> sure swap entries work as expected with below config as spotted by
> Alistair:
>
>   !CONFIG_MIGRATION &&
>   !CONFIG_MEMORY_FAILURE &&
>   !CONFIG_DEVICE_PRIVATE &&
>   CONFIG_PTE_MARKER
>
> (PS: this config should mostly never gonna happen, though, afaict..)
>
> Quoting the same thread [1] as above.
>
> Please review, thanks.
> ---
>  include/linux/swapops.h | 8 --------
>  1 file changed, 8 deletions(-)
>
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index fffbba0036f6..a291f210e7f8 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -493,18 +493,10 @@ static inline void num_poisoned_pages_inc(void)
>  }
>  #endif
>
> -#if defined(CONFIG_MEMORY_FAILURE) || defined(CONFIG_MIGRATION) || \
> -    defined(CONFIG_DEVICE_PRIVATE)
>  static inline int non_swap_entry(swp_entry_t entry)
>  {
>  	return swp_type(entry) >= MAX_SWAPFILES;
>  }
> -#else
> -static inline int non_swap_entry(swp_entry_t entry)
> -{
> -	return 0;
> -}
> -#endif
>
>  #endif /* CONFIG_MMU */
>  #endif /* _LINUX_SWAPOPS_H */