Re: [RFC PATCH v3 2/4] mm: distinguish large folio swap allocation failures

[Barry Song]

On Fri, Jul 17, 2026 at 8:25 PM Xueyuan Chen <xueyuan.chen21@xxxxxxxxx> wrote:
>
> folio_alloc_swap() reports most allocation failures with a generic
> negative error code. Reclaim cannot tell whether splitting a large folio
> could make progress or whether there is no backing space at all.
>
> Track the global free swap count around the allocation attempt and let the
> memcg swap charge path cap it by the remaining hierarchical swap margin.
> Return -E2BIG for large folios when a smaller allocation might still fit,
> -ENOSPC when no swap space is available, and -ENOMEM when the failure is
> not helped by splitting.
>
> This only refines folio_alloc_swap() return codes. The reclaim caller is
> updated separately.
>
> Signed-off-by: Xueyuan Chen <xueyuan.chen21@xxxxxxxxx>
> ---
>  include/linux/swap.h | 10 ++++++----
>  mm/memcontrol.c      | 10 +++++++++-
>  mm/swapfile.c        | 21 +++++++++++++++------
>  3 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 7a09df6977a5..0695ac56457f 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -571,13 +571,14 @@ static inline void folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
>  #endif
>
>  #if defined(CONFIG_MEMCG) && defined(CONFIG_SWAP)
> -int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry);
> +int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry,
> +                                long *nr_swap_pages);
>  static inline int mem_cgroup_try_charge_swap(struct folio *folio,
> -               swp_entry_t entry)
> +               swp_entry_t entry, long *nr_swap_pages)
>  {
>         if (mem_cgroup_disabled())
>                 return 0;
> -       return __mem_cgroup_try_charge_swap(folio, entry);
> +       return __mem_cgroup_try_charge_swap(folio, entry, nr_swap_pages);
>  }
>
>  extern void __mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages);
> @@ -592,7 +593,8 @@ extern long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg);
>  extern bool mem_cgroup_swap_full(struct folio *folio);
>  #else
>  static inline int mem_cgroup_try_charge_swap(struct folio *folio,
> -                                            swp_entry_t entry)
> +                                            swp_entry_t entry,
> +                                            long *nr_swap_pages)
>  {
>         return 0;
>  }
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1e10f493d2a9..7c9e7072fb39 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5472,12 +5472,14 @@ int __init mem_cgroup_init(void)
>   * __mem_cgroup_try_charge_swap - try charging swap space for a folio
>   * @folio: folio being added to swap
>   * @entry: swap entry to charge
> + * @nr_swap_pages: optional swap availability to cap by memcg margin
>   *
>   * Try to charge @folio's memcg for the swap space at @entry.
>   *
>   * Returns 0 on success, -ENOMEM on failure.
>   */
> -int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
> +int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry,
> +                                long *nr_swap_pages)
>  {
>         unsigned int nr_pages = folio_nr_pages(folio);
>         struct page_counter *counter;
> @@ -5495,6 +5497,9 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
>         rcu_read_lock();
>         memcg = obj_cgroup_memcg(objcg);
>         if (!entry.val) {
> +               if (nr_swap_pages && !mem_cgroup_is_root(memcg))
> +                       *nr_swap_pages = min(*nr_swap_pages,
> +                                            page_counter_margin(&memcg->swap));

This looks a bit odd. Could we decouple *nr_swap_pages from margin and
have the function always return margin instead?

I don't think this function needs to care about the initial value returned by
get_nr_swap_pages(). While folio_alloc_swap() may depend on it, they're
different software layers, and we shouldn't couple them together.


>                 memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
>                 rcu_read_unlock();
>                 return 0;
> @@ -5509,6 +5514,9 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
>                 memcg_memory_event(memcg, MEMCG_SWAP_MAX);
>                 memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
>                 mem_cgroup_private_id_put(memcg, nr_pages);
> +               if (nr_swap_pages)
> +                       *nr_swap_pages = min(*nr_swap_pages,
> +                                            page_counter_margin(counter));
>                 return -ENOMEM;
>         }
>         mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 9174f1eeffb0..53a921ca099a 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1690,12 +1690,14 @@ static int swap_dup_entries_cluster(struct swap_info_struct *si,
>   * swap cache.
>   *
>   * Context: Caller needs to hold the folio lock.
> - * Return: Whether the folio was added to the swap cache.
> + * Return: 0 on success, -E2BIG if splitting the folio might allow swapout,
> + * or another negative error code if splitting would not help.
>   */
>  int folio_alloc_swap(struct folio *folio)
>  {
>         unsigned int order = folio_order(folio);
>         unsigned int size = 1 << order;
> +       long nr_swap_pages;
>
>         VM_BUG_ON_FOLIO(!folio_test_locked(folio), folio);
>         VM_BUG_ON_FOLIO(!folio_test_uptodate(folio), folio);
> @@ -1706,7 +1708,7 @@ int folio_alloc_swap(struct folio *folio)
>                  * the caller should split the folio and try again.
>                  */
>                 if (!IS_ENABLED(CONFIG_THP_SWAP))
> -                       return -EAGAIN;
> +                       return -E2BIG;

We could also have the case where we're running out of swap space and it's not
worth splitting, even when THP_SWAP is disabled.

>
>                 /*
>                  * Allocation size should never exceed cluster size
> @@ -1714,10 +1716,12 @@ int folio_alloc_swap(struct folio *folio)
>                  */
>                 if (size > SWAPFILE_CLUSTER) {
>                         VM_WARN_ON_ONCE(1);
> -                       return -EINVAL;
> +                       return -E2BIG;

Similar to the above.

>                 }
>         }
>
> +       nr_swap_pages = get_nr_swap_pages();
> +
>  again:
>         local_lock(&percpu_swap_cluster.lock);
>         if (!swap_alloc_fast(folio))
> @@ -1730,11 +1734,16 @@ int folio_alloc_swap(struct folio *folio)
>         }
>
>         /* Need to call this even if allocation failed, for MEMCG_SWAP_FAIL. */
> -       if (unlikely(mem_cgroup_try_charge_swap(folio, folio->swap)))
> +       if (unlikely(mem_cgroup_try_charge_swap(folio, folio->swap,
> +                                               &nr_swap_pages))) {

As explained above, we shouldn't couple two different software layers.
don't use nr_swap_pages() as the initial value of memcg charge.

Best Regards
Barry