linux-kernel - Re: [PATCH v3 1/2] mm/filemap: use xas_try_split() in __filemap_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250308181402.95667-1-sj@kernel.org>
Date: Sat,  8 Mar 2025 10:14:02 -0800
From: SeongJae Park <sj@...nel.org>
To: Zi Yan <ziy@...dia.com>
Cc: SeongJae Park <sj@...nel.org>,
	Baolin Wang <baolin.wang@...ux.alibaba.com>,
	Matthew Wilcox <willy@...radead.org>,
	linux-mm@...ck.org,
	linux-fsdevel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	Kairui Song <kasong@...cent.com>,
	Miaohe Lin <linmiaohe@...wei.com>,
	linux-kernel@...r.kernel.org,
	David Hildenbrand <david@...hat.com>,
	John Hubbard <jhubbard@...dia.com>,
	Kefeng Wang <wangkefeng.wang@...wei.com>,
	"Kirill A. Shuemov" <kirill.shutemov@...ux.intel.com>,
	Ryan Roberts <ryan.roberts@....com>,
	Yang Shi <yang@...amperecomputing.com>,
	Yu Zhao <yuzhao@...gle.com>
Subject: Re: [PATCH v3 1/2] mm/filemap: use xas_try_split() in __filemap_add_folio()

Hello,

On Wed, 26 Feb 2025 16:08:53 -0500 Zi Yan <ziy@...dia.com> 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@...dia.com>
> Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
> Cc: Hugh Dickins <hughd@...gle.com>
> Cc: Kairui Song <kasong@...cent.com>
> Cc: Miaohe Lin <linmiaohe@...wei.com>
> Cc: Mattew Wilcox <willy@...radead.org>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: John Hubbard <jhubbard@...dia.com>
> Cc: Kefeng Wang <wangkefeng.wang@...wei.com>
> Cc: Kirill A. Shuemov <kirill.shutemov@...ux.intel.com>
> Cc: Ryan Roberts <ryan.roberts@....com>
> Cc: Yang Shi <yang@...amperecomputing.com>
> Cc: Yu Zhao <yuzhao@...gle.com>
> ---
>  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

[...]