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

Huang, Ying
2022-08-12T02:34:46Z

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);