Re: [Patch v3 2/2] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported()

[Wei Yang <richard.weiyang@gmail.com>]

On Thu, Nov 06, 2025 at 09:07:22PM -0500, Zi Yan wrote:
>On 6 Nov 2025, at 20:17, Wei Yang wrote:
>
>> On Thu, Nov 06, 2025 at 07:46:14PM -0500, Zi Yan wrote:
>>> On 5 Nov 2025, at 22:41, Wei Yang wrote:
>>>
>>>> The functions uniform_split_supported() and
>>>> non_uniform_split_supported() share significantly similar logic.
>>>>
>>>> The only functional difference is that uniform_split_supported()
>>>> includes an additional check on the requested @new_order.
>>>>
>>>> The reason for this check comes from the following two aspects:
>>>>
>>>>   * some file system or swap cache just supports order-0 folio
>>>>   * the behavioral difference between uniform/non-uniform split
>>>>
>>>> The behavioral difference between uniform split and non-uniform:
>>>>
>>>>   * uniform split splits folio directly to @new_order
>>>>   * non-uniform split creates after-split folios with orders from
>>>>     folio_order(folio) - 1 to new_order.
>>>>
>>>> This means for non-uniform split or !new_order split we should check the
>>>> file system and swap cache respectively.
>>>>
>>>> This commit unifies the logic and merge the two functions into a single
>>>> combined helper, removing redundant code and simplifying the split
>>>> support checking mechanism.
>>>>
>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>> Cc: Zi Yan <ziy@nvidia.com>
>>>> Cc: "David Hildenbrand (Red Hat)" <david@kernel.org>
>>>>
>>>> ---
>>>> v3:
>>>>   * adjust to use split_type
>>>>   * rebase on Zi Yan fix lkml.kernel.org/r/20251105162910.752266-1-ziy@nvidia.com
>>>> v2:
>>>>   * remove need_check
>>>>   * update comment
>>>>   * add more explanation in change log
>>>> ---
>>>>  include/linux/huge_mm.h |  8 ++---
>>>>  mm/huge_memory.c        | 71 +++++++++++++++++------------------------
>>>>  2 files changed, 33 insertions(+), 46 deletions(-)
>>>>
>>> LGTM. Thanks. Reviewed-by: Zi Yan <ziy@nvidia.com>
>>
>> Hi, Zi
>>
>> I am thinking whether it is proper to move the check (new_order < min_order)
>> from __folio_split() to folio_split_supported(). So that we could bail out
>> early if file system couldn't split to new_order.
>>
>> Not sure you like it or not.
>
>It sounds reasonable. My only concern is that that might add another
>indentation to the else branch in folio_split_supported().
>
>You can send a patch, so we can see how it looks.
>

Here is what come up my mind.

If !CONFIG_READ_ONLY_THP_FOR_FS, we directly compare new_order and min_order.

If CONFIG_READ_ONLY_THP_FOR_FS, one thing I am not sure is for the khugepaged
collapsed THP. If its min_order is 0, it looks we can cover it with following
check.

Look forward your insight.

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index dee416b3f6ed..ef05f246df73 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3704,8 +3704,8 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
 		if (new_order == 1)
 			return false;
 	} else if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order) {
-		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
-		    !mapping_large_folio_support(folio->mapping)) {
+		unsigned int min_order = mapping_min_folio_order(folio->mapping);
+		if (new_order < min_order) {
 			/*
 			 * We can always split a folio down to a single page
 			 * (new_order == 0) uniformly.
@@ -3827,7 +3827,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 		}
 		mapping = NULL;
 	} else {
-		unsigned int min_order;
 		gfp_t gfp;
 
 		mapping = folio->mapping;
@@ -3843,12 +3842,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 			goto out;
 		}
 
-		min_order = mapping_min_folio_order(folio->mapping);
-		if (new_order < min_order) {
-			ret = -EINVAL;
-			goto out;
-		}
-
 		gfp = current_gfp_context(mapping_gfp_mask(mapping) &
 							GFP_RECLAIM_MASK);
 
-- 
Wei Yang
Help you, Help me