Re: [PATCH v4 6/7] mm/swap: Cache maximum swapfile size when init swap

["Huang, Ying" <ying.huang@intel.com>]

Peter Xu <peterx@redhat.com> writes:

> We used to have swapfile_maximum_size() fetching a maximum value of
> swapfile size per-arch.
>
> As the caller of max_swapfile_size() grows, this patch introduce a variable
> "swapfile_maximum_size" and cache the value of old max_swapfile_size(), so
> that we don't need to calculate the value every time.
>
> Caching the value in swapfile_init() is safe because when reaching the
> phase we should have initialized all the relevant information.  Here the
> major arch to take care of is x86, which defines the max swapfile size
> based on L1TF mitigation.
>
> Here both X86_BUG_L1TF or l1tf_mitigation should have been setup properly
> when reaching swapfile_init(). As a reference, the code path looks like
> this for x86:
>
> - start_kernel
>   - setup_arch
>     - early_cpu_init
>       - early_identify_cpu --> setup X86_BUG_L1TF
>   - parse_early_param
>     - l1tf_cmdline --> set l1tf_mitigation
>   - check_bugs
>     - l1tf_select_mitigation --> set l1tf_mitigation
>   - arch_call_rest_init
>     - rest_init
>       - kernel_init
>         - kernel_init_freeable
>           - do_basic_setup
>             - do_initcalls --> calls swapfile_init() (initcall level 4)
>
> The swapfile size only depends on swp pte format on non-x86 archs, so
> caching it is safe too.
>
> Since at it, rename max_swapfile_size() to arch_max_swapfile_size() because
> arch can define its own function, so it's more straightforward to have
> "arch_" as its prefix.  At the meantime, export swapfile_maximum_size to
> replace the old usages of max_swapfile_size().
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

LGTM.

Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

Best Regards,
Huang, Ying

> ---
>  arch/x86/mm/init.c       | 2 +-
>  include/linux/swapfile.h | 3 ++-
>  include/linux/swapops.h  | 2 +-
>  mm/swapfile.c            | 7 +++++--
>  4 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index 82a042c03824..9121bc1b9453 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -1054,7 +1054,7 @@ void update_cache_mode_entry(unsigned entry, enum page_cache_mode cache)
>  }
>  
>  #ifdef CONFIG_SWAP
> -unsigned long max_swapfile_size(void)
> +unsigned long arch_max_swapfile_size(void)
>  {
>  	unsigned long pages;
>  
> diff --git a/include/linux/swapfile.h b/include/linux/swapfile.h
> index 54078542134c..165e0bd04862 100644
> --- a/include/linux/swapfile.h
> +++ b/include/linux/swapfile.h
> @@ -8,6 +8,7 @@
>   */
>  extern struct swap_info_struct *swap_info[];
>  extern unsigned long generic_max_swapfile_size(void);
> -extern unsigned long max_swapfile_size(void);
> +/* Maximum swapfile size supported for the arch (not inclusive). */
> +extern unsigned long swapfile_maximum_size;
>  
>  #endif /* _LINUX_SWAPFILE_H */
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index 36e462e116af..f25b566643f1 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -307,7 +307,7 @@ static inline bool migration_entry_supports_ad(void)
>  	 * the offset large enough to cover all of them (PFN, A & D bits).
>  	 */
>  #ifdef CONFIG_SWAP
> -	return max_swapfile_size() >= (1UL << SWP_MIG_TOTAL_BITS);
> +	return swapfile_maximum_size >= (1UL << SWP_MIG_TOTAL_BITS);
>  #else  /* CONFIG_SWAP */
>  	return false;
>  #endif	/* CONFIG_SWAP */
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 1fdccd2f1422..3cc64399df44 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -63,6 +63,7 @@ EXPORT_SYMBOL_GPL(nr_swap_pages);
>  /* protected with swap_lock. reading in vm_swap_full() doesn't need lock */
>  long total_swap_pages;
>  static int least_priority = -1;
> +unsigned long swapfile_maximum_size;
>  
>  static const char Bad_file[] = "Bad swap file entry ";
>  static const char Unused_file[] = "Unused swap file entry ";
> @@ -2816,7 +2817,7 @@ unsigned long generic_max_swapfile_size(void)
>  }
>  
>  /* Can be overridden by an architecture for additional checks. */
> -__weak unsigned long max_swapfile_size(void)
> +__weak unsigned long arch_max_swapfile_size(void)
>  {
>  	return generic_max_swapfile_size();
>  }
> @@ -2856,7 +2857,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,
>  	p->cluster_next = 1;
>  	p->cluster_nr = 0;
>  
> -	maxpages = max_swapfile_size();
> +	maxpages = swapfile_maximum_size;
>  	last_page = swap_header->info.last_page;
>  	if (!last_page) {
>  		pr_warn("Empty swap-file\n");
> @@ -3677,6 +3678,8 @@ static int __init swapfile_init(void)
>  	for_each_node(nid)
>  		plist_head_init(&swap_avail_heads[nid]);
>  
> +	swapfile_maximum_size = arch_max_swapfile_size();
> +
>  	return 0;
>  }
>  subsys_initcall(swapfile_init);