Re: [PATCH v3 1/2] mm/filemap: use xas_try_split() in __filemap_add_folio()

[SeongJae Park]

Hello,

On Wed, 26 Feb 2025 16:08:53 -0500 Zi Yan <ziy@xxxxxxxxxx> wrote:

> During __filemap_add_folio(), a shadow entry is covering n slots and a
> folio covers m slots with m < n is to be added.  Instead of splitting all
> n slots, only the m slots covered by the folio need to be split and the
> remaining n-m shadow entries can be retained with orders ranging from m to
> n-1.  This method only requires
> 
> 	(n/XA_CHUNK_SHIFT) - (m/XA_CHUNK_SHIFT)
> 
> new xa_nodes instead of
> 	(n % XA_CHUNK_SHIFT) * ((n/XA_CHUNK_SHIFT) - (m/XA_CHUNK_SHIFT))
> 
> new xa_nodes, compared to the original xas_split_alloc() + xas_split()
> one.  For example, to insert an order-0 folio when an order-9 shadow entry
> is present (assuming XA_CHUNK_SHIFT is 6), 1 xa_node is needed instead of
> 8.
> 
> xas_try_split_min_order() is introduced to reduce the number of calls to
> xas_try_split() during split.
> 
> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> Cc: Kairui Song <kasong@xxxxxxxxxxx>
> Cc: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> Cc: Mattew Wilcox <willy@xxxxxxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>
> Cc: John Hubbard <jhubbard@xxxxxxxxxx>
> Cc: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> Cc: Kirill A. Shuemov <kirill.shutemov@xxxxxxxxxxxxxxx>
> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
> Cc: Yang Shi <yang@xxxxxxxxxxxxxxxxxxxxxx>
> Cc: Yu Zhao <yuzhao@xxxxxxxxxx>
> ---
>  include/linux/xarray.h |  7 +++++++
>  lib/xarray.c           | 25 +++++++++++++++++++++++
>  mm/filemap.c           | 45 +++++++++++++++++-------------------------
>  3 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/include/linux/xarray.h b/include/linux/xarray.h
> index 4010195201c9..78eede109b1a 100644
> --- a/include/linux/xarray.h
> +++ b/include/linux/xarray.h
> @@ -1556,6 +1556,7 @@ int xas_get_order(struct xa_state *xas);
>  void xas_split(struct xa_state *, void *entry, unsigned int order);
>  void xas_split_alloc(struct xa_state *, void *entry, unsigned int order, gfp_t);
>  void xas_try_split(struct xa_state *xas, void *entry, unsigned int order);
> +unsigned int xas_try_split_min_order(unsigned int order);
>  #else
>  static inline int xa_get_order(struct xarray *xa, unsigned long index)
>  {
> @@ -1582,6 +1583,12 @@ static inline void xas_try_split(struct xa_state *xas, void *entry,
>  		unsigned int order)
>  {
>  }
> +
> +static inline unsigned int xas_try_split_min_order(unsigned int order)
> +{
> +	return 0;
> +}
> +
>  #endif
>  
>  /**
> diff --git a/lib/xarray.c b/lib/xarray.c
> index bc197c96d171..8067182d3e43 100644
> --- a/lib/xarray.c
> +++ b/lib/xarray.c
> @@ -1133,6 +1133,28 @@ void xas_split(struct xa_state *xas, void *entry, unsigned int order)
>  }
>  EXPORT_SYMBOL_GPL(xas_split);
>  
> +/**
> + * xas_try_split_min_order() - Minimal split order xas_try_split() can accept
> + * @order: Current entry order.
> + *
> + * xas_try_split() can split a multi-index entry to smaller than @order - 1 if
> + * no new xa_node is needed. This function provides the minimal order
> + * xas_try_split() supports.
> + *
> + * Return: the minimal order xas_try_split() supports
> + *
> + * Context: Any context.
> + *
> + */
> +unsigned int xas_try_split_min_order(unsigned int order)
> +{
> +	if (order % XA_CHUNK_SHIFT == 0)
> +		return order == 0 ? 0 : order - 1;
> +
> +	return order - (order % XA_CHUNK_SHIFT);
> +}
> +EXPORT_SYMBOL_GPL(xas_try_split_min_order);
> +

I found this makes build fails when CONFIG_XARRAY_MULTI is unset, like below.

    /linux/lib/xarray.c:1251:14: error: redefinition of ‘xas_try_split_min_order’
     1251 | unsigned int xas_try_split_min_order(unsigned int order)
          |              ^~~~~~~~~~~~~~~~~~~~~~~
    In file included from /linux/lib/xarray.c:13:
    /linux/include/linux/xarray.h:1587:28: note: previous definition of ‘xas_try_split_min_order’ with type ‘unsigned int(unsigned int)’
     1587 | static inline unsigned int xas_try_split_min_order(unsigned int order)
          |                            ^~~~~~~~~~~~~~~~~~~~~~~

I think we should have the definition only when CONFIG_XARRAY_MULTI?


Thanks,
SJ

[...]