[v9,04/10] mm: split a folio in minimum folio order chunks

Commit Message

Pankaj Raghav (Samsung) July 4, 2024, 11:23 a.m. UTC

From: Luis Chamberlain <[e-mail]>

split_folio() and split_folio_to_list() assume order 0, to support
minorder for non-anonymous folios, we must expand these to check the
folio mapping order and use that.

Set new_order to be at least minimum folio order if it is set in
split_huge_page_to_list() so that we can maintain minimum folio order
requirement in the page cache.

Update the debugfs write files used for testing to ensure the order
is respected as well. We simply enforce the min order when a file
mapping is used.

Signed-off-by: Luis Chamberlain <[e-mail]>
Signed-off-by: Pankaj Raghav <[e-mail]>
Reviewed-by: Hannes Reinecke <[e-mail]>
---
 include/linux/huge_mm.h | 14 ++++++++---
 mm/huge_memory.c        | 55 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 61 insertions(+), 8 deletions(-)

Comments

Zi Yan July 8, 2024, 1:53 p.m. UTC | #1

On Thu Jul 4, 2024 at 7:23 AM EDT, Pankaj Raghav (Samsung) wrote:
> From: Luis Chamberlain <[e-mail]>
>
> split_folio() and split_folio_to_list() assume order 0, to support
> minorder for non-anonymous folios, we must expand these to check the
> folio mapping order and use that.
>
> Set new_order to be at least minimum folio order if it is set in
> split_huge_page_to_list() so that we can maintain minimum folio order
> requirement in the page cache.
>
> Update the debugfs write files used for testing to ensure the order
> is respected as well. We simply enforce the min order when a file
> mapping is used.
>
> Signed-off-by: Luis Chamberlain <[e-mail]>
> Signed-off-by: Pankaj Raghav <[e-mail]>
> Reviewed-by: Hannes Reinecke <[e-mail]>
> ---
>  include/linux/huge_mm.h | 14 ++++++++---
>  mm/huge_memory.c        | 55 ++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 61 insertions(+), 8 deletions(-)

<snip>
>
> @@ -3265,6 +3277,21 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
>  	return ret;
>  }
>  
> +int split_folio_to_list(struct folio *folio, struct list_head *list)
> +{
> +	unsigned int min_order = 0;
> +
> +	if (!folio_test_anon(folio)) {
> +		if (!folio->mapping && folio_test_pmd_mappable(folio)) {
> +			count_vm_event(THP_SPLIT_PAGE_FAILED);
> +			return -EBUSY;
> +		}

This should be

		if (!folio->mapping) {
			if (folio_test_pmd_mappable(folio))
				count_vm_event(THP_SPLIT_PAGE_FAILED);
			return -EBUSY;
		}

Otherwise, a non PMD mappable folio with no mapping will fall through
and cause NULL pointer dereference in mapping_min_folio_order().

> +		min_order = mapping_min_folio_order(folio->mapping);
> +	}
> +
> +	return split_huge_page_to_list_to_order(&folio->page, list, min_order);
> +}
> +
>  void __folio_undo_large_rmappable(struct folio *folio)
>  {
>  	struct deferred_split *ds_queue;

Pankaj Raghav (Samsung) July 9, 2024, 11:04 a.m. UTC | #2

> 
> This should be
> 
> 		if (!folio->mapping) {
> 			if (folio_test_pmd_mappable(folio))
> 				count_vm_event(THP_SPLIT_PAGE_FAILED);
> 			return -EBUSY;
> 		}
> 
> Otherwise, a non PMD mappable folio with no mapping will fall through
> and cause NULL pointer dereference in mapping_min_folio_order().

Ah, of course. I thought I was being "smart" here to avoid another
nesting. Instead of triple nested ifs, I guess this is better:

int split_folio_to_list(struct folio *folio, struct list_head *list)
{
       unsigned int min_order = 0;

       if (folio_test_anon(folio))
               goto out;

       if (!folio->mapping) {
               if (folio_test_pmd_mappable(folio))
                       count_vm_event(THP_SPLIT_PAGE_FAILED);
               return -EBUSY;
       }

       min_order = mapping_min_folio_order(folio->mapping);
out:
       return split_huge_page_to_list_to_order(&folio->page, list,
                                                       min_order);
}

Let me know what you think!

> 
> > +		min_order = mapping_min_folio_order(folio->mapping);
> > +	}
> > +
> > +	return split_huge_page_to_list_to_order(&folio->page, list, min_order);
> > +}
> > +
> >  void __folio_undo_large_rmappable(struct folio *folio)
> >  {
> >  	struct deferred_split *ds_queue;
> 
> 
> -- 
> Best Regards,
> Yan, Zi
>

Zi Yan July 9, 2024, 12:49 p.m. UTC | #3

On 9 Jul 2024, at 7:04, Pankaj Raghav (Samsung) wrote:

>>
>> This should be
>>
>> 		if (!folio->mapping) {
>> 			if (folio_test_pmd_mappable(folio))
>> 				count_vm_event(THP_SPLIT_PAGE_FAILED);
>> 			return -EBUSY;
>> 		}
>>
>> Otherwise, a non PMD mappable folio with no mapping will fall through
>> and cause NULL pointer dereference in mapping_min_folio_order().
>
> Ah, of course. I thought I was being "smart" here to avoid another
> nesting. Instead of triple nested ifs, I guess this is better:
>
> int split_folio_to_list(struct folio *folio, struct list_head *list)
> {
>        unsigned int min_order = 0;
>
>        if (folio_test_anon(folio))
>                goto out;
>
>        if (!folio->mapping) {
>                if (folio_test_pmd_mappable(folio))
>                        count_vm_event(THP_SPLIT_PAGE_FAILED);
>                return -EBUSY;
>        }
>
>        min_order = mapping_min_folio_order(folio->mapping);
> out:
>        return split_huge_page_to_list_to_order(&folio->page, list,
>                                                        min_order);
> }
>
> Let me know what you think!

LGTM. Thanks. With this change, feel free to add Reviewed-by: Zi Yan <[e-mail]>

--
Best Regards,
Yan, Zi

Patch

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 4d155c7a47922..b320a246293ec 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -90,6 +90,8 @@  extern struct kobj_attribute thpsize_shmem_enabled_attr;
 #define thp_vma_allowable_order(vma, vm_flags, tva_flags, order) \
 	(!!thp_vma_allowable_orders(vma, vm_flags, tva_flags, BIT(order)))
 
+#define split_folio(f) split_folio_to_list(f, NULL)
+
 #ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
 #define HPAGE_PMD_SHIFT PMD_SHIFT
 #define HPAGE_PUD_SHIFT PUD_SHIFT
@@ -327,9 +329,10 @@  unsigned long thp_get_unmapped_area_vmflags(struct file *filp, unsigned long add
 bool can_split_folio(struct folio *folio, int *pextra_pins);
 int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 		unsigned int new_order);
+int split_folio_to_list(struct folio *folio, struct list_head *list);
 static inline int split_huge_page(struct page *page)
 {
-	return split_huge_page_to_list_to_order(page, NULL, 0);
+	return split_folio(page_folio(page));
 }
 void deferred_split_folio(struct folio *folio);
 
@@ -501,6 +504,12 @@  static inline int split_huge_page(struct page *page)
 {
 	return 0;
 }
+
+static inline int split_folio_to_list(struct folio *folio, struct list_head *list)
+{
+	return 0;
+}
+
 static inline void deferred_split_folio(struct folio *folio) {}
 #define split_huge_pmd(__vma, __pmd, __address)	\
 	do { } while (0)
@@ -615,7 +624,4 @@  static inline int split_folio_to_order(struct folio *folio, int new_order)
 	return split_folio_to_list_to_order(folio, NULL, new_order);
 }
 
-#define split_folio_to_list(f, l) split_folio_to_list_to_order(f, l, 0)
-#define split_folio(f) split_folio_to_order(f, 0)
-
 #endif /* _LINUX_HUGE_MM_H */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index a633206375af1..2fd45d8730bab 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3062,6 +3062,9 @@  bool can_split_folio(struct folio *folio, int *pextra_pins)
  * released, or if some unexpected race happened (e.g., anon VMA disappeared,
  * truncation).
  *
+ * Callers should ensure that the order respects the address space mapping
+ * min-order if one is set for non-anonymous folios.
+ *
  * Returns -EINVAL when trying to split to an order that is incompatible
  * with the folio. Splitting to order 0 is compatible with all folios.
  */
@@ -3143,6 +3146,7 @@  int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 		mapping = NULL;
 		anon_vma_lock_write(anon_vma);
 	} else {
+		unsigned int min_order;
 		gfp_t gfp;
 
 		mapping = folio->mapping;
@@ -3153,6 +3157,14 @@  int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 			goto out;
 		}
 
+		min_order = mapping_min_folio_order(folio->mapping);
+		if (new_order < min_order) {
+			VM_WARN_ONCE(1, "Cannot split mapped folio below min-order: %u",
+				     min_order);
+			ret = -EINVAL;
+			goto out;
+		}
+
 		gfp = current_gfp_context(mapping_gfp_mask(mapping) &
 							GFP_RECLAIM_MASK);
 
@@ -3265,6 +3277,21 @@  int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 	return ret;
 }
 
+int split_folio_to_list(struct folio *folio, struct list_head *list)
+{
+	unsigned int min_order = 0;
+
+	if (!folio_test_anon(folio)) {
+		if (!folio->mapping && folio_test_pmd_mappable(folio)) {
+			count_vm_event(THP_SPLIT_PAGE_FAILED);
+			return -EBUSY;
+		}
+		min_order = mapping_min_folio_order(folio->mapping);
+	}
+
+	return split_huge_page_to_list_to_order(&folio->page, list, min_order);
+}
+
 void __folio_undo_large_rmappable(struct folio *folio)
 {
 	struct deferred_split *ds_queue;
@@ -3496,6 +3523,8 @@  static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
 		struct vm_area_struct *vma = vma_lookup(mm, addr);
 		struct page *page;
 		struct folio *folio;
+		struct address_space *mapping;
+		unsigned int target_order = new_order;
 
 		if (!vma)
 			break;
@@ -3516,7 +3545,13 @@  static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
 		if (!is_transparent_hugepage(folio))
 			goto next;
 
-		if (new_order >= folio_order(folio))
+		if (!folio_test_anon(folio)) {
+			mapping = folio->mapping;
+			target_order = max(new_order,
+					   mapping_min_folio_order(mapping));
+		}
+
+		if (target_order >= folio_order(folio))
 			goto next;
 
 		total++;
@@ -3532,9 +3567,13 @@  static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
 		if (!folio_trylock(folio))
 			goto next;
 
-		if (!split_folio_to_order(folio, new_order))
+		if (!folio_test_anon(folio) && folio->mapping != mapping)
+			goto unlock;
+
+		if (!split_folio_to_order(folio, target_order))
 			split++;
 
+unlock:
 		folio_unlock(folio);
 next:
 		folio_put(folio);
@@ -3559,6 +3598,7 @@  static int split_huge_pages_in_file(const char *file_path, pgoff_t off_start,
 	pgoff_t index;
 	int nr_pages = 1;
 	unsigned long total = 0, split = 0;
+	unsigned int min_order;
 
 	file = getname_kernel(file_path);
 	if (IS_ERR(file))
@@ -3572,9 +3612,11 @@  static int split_huge_pages_in_file(const char *file_path, pgoff_t off_start,
 		 file_path, off_start, off_end);
 
 	mapping = candidate->f_mapping;
+	min_order = mapping_min_folio_order(mapping);
 
 	for (index = off_start; index < off_end; index += nr_pages) {
 		struct folio *folio = filemap_get_folio(mapping, index);
+		unsigned int target_order = new_order;
 
 		nr_pages = 1;
 		if (IS_ERR(folio))
@@ -3583,18 +3625,23 @@  static int split_huge_pages_in_file(const char *file_path, pgoff_t off_start,
 		if (!folio_test_large(folio))
 			goto next;
 
+		target_order = max(new_order, min_order);
 		total++;
 		nr_pages = folio_nr_pages(folio);
 
-		if (new_order >= folio_order(folio))
+		if (target_order >= folio_order(folio))
 			goto next;
 
 		if (!folio_trylock(folio))
 			goto next;
 
-		if (!split_folio_to_order(folio, new_order))
+		if (folio->mapping != mapping)
+			goto unlock;
+
+		if (!split_folio_to_order(folio, target_order))
 			split++;
 
+unlock:
 		folio_unlock(folio);
 next:
 		folio_put(folio);