[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20A1553F-C30A-4D93-8A43-011163A22C60@nvidia.com>
Date: Sat, 08 Mar 2025 13:32:02 -0500
From: Zi Yan <ziy@...dia.com>
To: SeongJae Park <sj@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>
Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>,
Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
linux-fsdevel@...r.kernel.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()
On 8 Mar 2025, at 13:14, SeongJae Park wrote:
> 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?
I think it might be a merge issue, since my original patch[1] places
xas_try_split_min_order() above xas_try_split(), both of which are
in #ifdef CONFIG_XARRAY_MULTI #endif. But mm-everything-2025-03-08-00-43
seems to move xas_try_split_min_order() below xas_try_split() and
out of CONFIG_XARRAY_MULTI guard.
[1] https://lore.kernel.org/linux-mm/20250226210854.2045816-2-ziy@nvidia.com/
--
Best Regards,
Yan, Zi
Powered by blists - more mailing lists