On Wed, Nov 11, 2020 at 03:40:03PM -0500, Zi Yan wrote:

From: Zi Yan ziy@nvidia.com

Huge pages in the process with the given pid and virtual address range are split. It is used to test split huge page function. In addition, a testing program is added to tools/testing/selftests/vm to utilize the interface by splitting PMD THPs.

Signed-off-by: Zi Yan ziy@nvidia.com

mm/huge_memory.c | 98 +++++++++++ mm/internal.h | 1 + mm/migrate.c | 2 +- tools/testing/selftests/vm/Makefile | 1 + .../selftests/vm/split_huge_page_test.c | 161 ++++++++++++++++++ 5 files changed, 262 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/vm/split_huge_page_test.c

diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 207ebca8c654..c4fead5ead31 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -7,6 +7,7 @@ #include <linux/mm.h> #include <linux/sched.h> +#include <linux/sched/mm.h> #include <linux/sched/coredump.h> #include <linux/sched/numa_balancing.h> #include <linux/highmem.h> @@ -2935,10 +2936,107 @@ static int split_huge_pages_set(void *data, u64 val) DEFINE_DEBUGFS_ATTRIBUTE(split_huge_pages_fops, NULL, split_huge_pages_set, "%llu\n"); +static ssize_t split_huge_pages_in_range_pid_write(struct file *file,

• const char __user *buf, size_t count, loff_t *ppops)
  

+{

• static DEFINE_MUTEX(mutex);
• ssize_t ret;
• char input_buf[80]; /* hold pid, start_vaddr, end_vaddr */
• int pid;
• unsigned long vaddr_start, vaddr_end, addr;
• nodemask_t task_nodes;
• struct mm_struct *mm;
• ret = mutex_lock_interruptible(&mutex);
• if (ret)

• return ret;
  

• ret = -EFAULT;
• memset(input_buf, 0, 80);
• if (copy_from_user(input_buf, buf, min_t(size_t, count, 80)))

• goto out;
  

• input_buf[80] = '\0';

Hm. Out-of-buffer access?

• ret = sscanf(input_buf, "%d,%lx,%lx", &pid, &vaddr_start, &vaddr_end);

Why hex without 0x prefix?

• if (ret != 3) {

• ret = -EINVAL;
  

• goto out;
  

• }
• vaddr_start &= PAGE_MASK;
• vaddr_end &= PAGE_MASK;
• ret = strlen(input_buf);
• pr_debug("split huge pages in pid: %d, vaddr: [%lx - %lx]\n",

•  pid, vaddr_start, vaddr_end);
  

• mm = find_mm_struct(pid, &task_nodes);

I don't follow why you need nodemask.

• if (IS_ERR(mm)) {

• ret = -EINVAL;
  

• goto out;
  

• }
• mmap_read_lock(mm);
• for (addr = vaddr_start; addr < vaddr_end;) {

• struct vm_area_struct *vma = find_vma(mm, addr);
  

• unsigned int follflags;
  

• struct page *page;
  

• if (!vma || addr < vma->vm_start || !vma_migratable(vma))
  

• 	break;
  

• /* FOLL_DUMP to ignore special (like zero) pages */
  

• follflags = FOLL_GET | FOLL_DUMP;
  

• page = follow_page(vma, addr, follflags);
  

• if (IS_ERR(page))
  

• 	break;
  

• if (!page)
  

• 	break;
  

• if (!is_transparent_hugepage(page))
  

• 	goto next;
  

• if (!can_split_huge_page(page, NULL))
  

• 	goto next;
  

• if (!trylock_page(page))
  

• 	goto next;
  

• addr += page_size(page) - PAGE_SIZE;
  

Who said it was mapped as huge? mremap() allows to construct an PTE page table that filled with PTE-mapped THPs, each of them distinct.

• /* reset addr if split fails */
  

• if (split_huge_page(page))
  

• 	addr -= (page_size(page) - PAGE_SIZE);
  

• unlock_page(page);
  

+next:

• /* next page */
  

• addr += page_size(page);
  

Isn't it the second time if split_huge_page() succeed.

• put_page(page);
  

• }
• mmap_read_unlock(mm);
• mmput(mm);

+out:

• mutex_unlock(&mutex);
• return ret;

+}

+static const struct file_operations split_huge_pages_in_range_pid_fops = {

• .owner = THIS_MODULE,
• .write = split_huge_pages_in_range_pid_write,
• .llseek = no_llseek,

+};

static int __init split_huge_pages_debugfs(void) { debugfs_create_file("split_huge_pages", 0200, NULL, NULL, &split_huge_pages_fops);

• debugfs_create_file("split_huge_pages_in_range_pid", 0200, NULL, NULL,

• 	    &split_huge_pages_in_range_pid_fops);
  

  return 0;

} late_initcall(split_huge_pages_debugfs); diff --git a/mm/internal.h b/mm/internal.h index 3ea43642b99d..fd841a38830f 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -624,4 +624,5 @@ struct migration_target_control { bool truncate_inode_partial_page(struct page *page, loff_t start, loff_t end); void page_cache_free_page(struct address_space *mapping, struct page *page); +struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes); #endif /* __MM_INTERNAL_H */ diff --git a/mm/migrate.c b/mm/migrate.c index a50bbb0e029b..e35654d1087d 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1851,7 +1851,7 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages, return nr_pages ? -EFAULT : 0; } -static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes) +struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes) { struct task_struct *task; struct mm_struct *mm; diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile index 62fb15f286ee..d9ead0cdd3e9 100644 --- a/tools/testing/selftests/vm/Makefile +++ b/tools/testing/selftests/vm/Makefile @@ -42,6 +42,7 @@ TEST_GEN_FILES += on-fault-limit TEST_GEN_FILES += thuge-gen TEST_GEN_FILES += transhuge-stress TEST_GEN_FILES += userfaultfd +TEST_GEN_FILES += split_huge_page_test ifeq ($(ARCH),x86_64) CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_32bit_program.c -m32) diff --git a/tools/testing/selftests/vm/split_huge_page_test.c b/tools/testing/selftests/vm/split_huge_page_test.c new file mode 100644 index 000000000000..c8a32ae9e13a --- /dev/null +++ b/tools/testing/selftests/vm/split_huge_page_test.c @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: GPL-2.0

+#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include "numa.h" +#include <unistd.h> +#include <errno.h> +#include <inttypes.h> +#include <string.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <sys/time.h> +#include <sys/wait.h> +#include <malloc.h> +#include <stdbool.h>

+#define PAGE_4KB (4096UL) +#define PAGE_2MB (512UL*PAGE_4KB) +#define PAGE_1GB (512UL*PAGE_2MB)

+#define PRESENT_MASK (1UL<<63) +#define SWAPPED_MASK (1UL<<62) +#define PAGE_TYPE_MASK (1UL<<61) +#define PFN_MASK ((1UL<<55)-1)

+#define KPF_THP (1UL<<22) +#define KPF_PUD_THP (1UL<<27)

+#define SPLIT_DEBUGFS "/sys/kernel/debug/split_huge_pages_in_range_pid" +#define SMAP_PATH "/proc/self/smaps" +#define INPUT_MAX 80

+static int write_file(const char *path, const char *buf, size_t buflen) +{

• int fd;
• ssize_t numwritten;
• fd = open(path, O_WRONLY);
• if (fd == -1)

• return 0;
  

• numwritten = write(fd, buf, buflen - 1);
• close(fd);
• if (numwritten < 1)

• return 0;
  

• return (unsigned int) numwritten;

+}

+static void write_debugfs(int pid, uint64_t vaddr_start, uint64_t vaddr_end) +{

• char input[INPUT_MAX];
• int ret;
• ret = snprintf(input, INPUT_MAX, "%d,%lx,%lx", pid, vaddr_start,

• 	vaddr_end);
  

• if (ret >= INPUT_MAX) {

• printf("%s: Debugfs input is too long\n", __func__);
  

• exit(EXIT_FAILURE);
  

• }
• if (!write_file(SPLIT_DEBUGFS, input, ret + 1)) {

• perror(SPLIT_DEBUGFS);
  

• exit(EXIT_FAILURE);
  

• }

+}

+#define MAX_LINE_LENGTH 500

+static bool check_for_pattern(FILE *fp, char *pattern, char *buf) +{

• while (fgets(buf, MAX_LINE_LENGTH, fp) != NULL) {

• if (!strncmp(buf, pattern, strlen(pattern)))
  

• 	return true;
  

• }
• return false;

+}

+static uint64_t check_huge(void *addr) +{

• uint64_t thp = 0;
• int ret;
• FILE *fp;
• char buffer[MAX_LINE_LENGTH];
• char addr_pattern[MAX_LINE_LENGTH];
• ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08lx-",

•        (unsigned long) addr);
  

• if (ret >= MAX_LINE_LENGTH) {

• printf("%s: Pattern is too long\n", __func__);
  

• exit(EXIT_FAILURE);
  

• }
• fp = fopen(SMAP_PATH, "r");
• if (!fp) {

• printf("%s: Failed to open file %s\n", __func__, SMAP_PATH);
  

• exit(EXIT_FAILURE);
  

• }
• if (!check_for_pattern(fp, addr_pattern, buffer))

• goto err_out;
  

• /*

• * Fetch the AnonHugePages: in the same block and check the number of
  

• * hugepages.
  

• */
  

• if (!check_for_pattern(fp, "AnonHugePages:", buffer))

• goto err_out;
  

• if (sscanf(buffer, "AnonHugePages:%10ld kB", &thp) != 1) {

• printf("Reading smap error\n");
  

• exit(EXIT_FAILURE);
  

• }

+err_out:

• fclose(fp);
• return thp;

+}

+void split_pmd_thp(void) +{

• char *one_page;
• size_t len = 4 * PAGE_2MB;
• uint64_t thp_size;
• one_page = memalign(PAGE_1GB, len);
• madvise(one_page, len, MADV_HUGEPAGE);
• memset(one_page, 1, len);
• thp_size = check_huge(one_page);
• if (!thp_size) {

• printf("No THP is allocatd");
  

• exit(EXIT_FAILURE);
  

• }
• /* split all possible huge pages */
• write_debugfs(getpid(), (uint64_t)one_page, (uint64_t)one_page + len);
• *one_page = 0;
• thp_size = check_huge(one_page);
• if (thp_size) {

• printf("Still %ld kB AnonHugePages not split\n", thp_size);
  

• exit(EXIT_FAILURE);
  

• }
• printf("Split huge pages successful\n");
• free(one_page);

+}

+int main(int argc, char **argv) +{

• split_pmd_thp();
• return 0;

+}

2.28.0

On 13 Nov 2020, at 19:23, Roman Gushchin wrote:

On Wed, Nov 11, 2020 at 03:40:04PM -0500, Zi Yan wrote:

...

From: Zi Yan ziy@nvidia.com

It reads thp_nr_pages and splits to provided new_nr. It prepares for upcoming changes to support split huge page to any lower order.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/memcontrol.h | 5 +++-- mm/huge_memory.c | 2 +- mm/memcontrol.c | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 0f4dd7829fb2..b3bac79ceed6 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -1105,7 +1105,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, }

#ifdef CONFIG_TRANSPARENT_HUGEPAGE -void mem_cgroup_split_huge_fixup(struct page *head); +void mem_cgroup_split_huge_fixup(struct page *head, unsigned int new_nr); #endif

#else /* CONFIG_MEMCG */ @@ -1451,7 +1451,8 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order, return 0; }

-static inline void mem_cgroup_split_huge_fixup(struct page *head) +static inline void mem_cgroup_split_huge_fixup(struct page *head,

• 			       unsigned int new_nr)
  

{ }

diff --git a/mm/huge_memory.c b/mm/huge_memory.c index c4fead5ead31..f599f5b9bf7f 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2429,7 +2429,7 @@ static void __split_huge_page(struct page *page, struct list_head *list, lruvec = mem_cgroup_page_lruvec(head, pgdat);

/* complete memcg works before add pages to LRU */

• mem_cgroup_split_huge_fixup(head);

• mem_cgroup_split_huge_fixup(head, 1);

  if (PageAnon(head) && PageSwapCache(head)) { swp_entry_t entry = { .val = page_private(head) };

diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 33f632689cee..e9705ba6bbcc 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -3247,7 +3247,7 @@ void obj_cgroup_uncharge(struct obj_cgroup *objcg, size_t size)

• Because tail pages are not marked as "used", set it. We're under
• pgdat->lru_lock and migration entries setup in all page mappings.

*/ -void mem_cgroup_split_huge_fixup(struct page *head) +void mem_cgroup_split_huge_fixup(struct page *head, unsigned int new_nr)

I'd go with unsigned int new_order, then it's obvious that we can split the original page without any leftovers.

Makes sense. Will change it.

Other than that the patch looks good! Acked-by: Roman Gushchin guro@fb.com

Thanks.

...

{ struct mem_cgroup *memcg = page_memcg(head); int i; @@ -3255,7 +3255,7 @@ void mem_cgroup_split_huge_fixup(struct page *head) if (mem_cgroup_disabled()) return;

• for (i = 1; i < thp_nr_pages(head); i++) {

• for (i = new_nr; i < thp_nr_pages(head); i += new_nr) { css_get(&memcg->css); head[i].memcg_data = (unsigned long)memcg; }

-- 2.28.0

— Best Regards, Yan Zi

On 11/11/20 12:40 PM, Zi Yan wrote:

From: Zi Yan ziy@nvidia.com

It adds a new_order parameter to set new page order in page owner. It prepares for upcoming changes to support split huge page to any lower order.

Signed-off-by: Zi Yan ziy@nvidia.com

Except for a minor fix below, you can add: Reviewed-by: Ralph Campbell rcampbell@nvidia.com

---

include/linux/page_owner.h | 7 ++++--- mm/huge_memory.c | 2 +- mm/page_alloc.c | 2 +- mm/page_owner.c | 6 +++--- 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h index 3468794f83d2..215cbb159568 100644 --- a/include/linux/page_owner.h +++ b/include/linux/page_owner.h @@ -31,10 +31,11 @@ static inline void set_page_owner(struct page *page, __set_page_owner(page, order, gfp_mask); } -static inline void split_page_owner(struct page *page, unsigned int nr) +static inline void split_page_owner(struct page *page, unsigned int nr,

• 	unsigned int new_order)
  

  { if (static_branch_unlikely(&page_owner_inited))

• __split_page_owner(page, nr);
  

• __split_page_owner(page, nr, new_order);
  

  } static inline void copy_page_owner(struct page *oldpage, struct page *newpage) {

@@ -60,7 +61,7 @@ static inline void set_page_owner(struct page *page, { } static inline void split_page_owner(struct page *page,

• 	unsigned int order)
  

• 	unsigned int nr, unsigned int new_order)
  

  { } static inline void copy_page_owner(struct page *oldpage, struct page *newpage)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c index f599f5b9bf7f..8b7d771ee962 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2459,7 +2459,7 @@ static void __split_huge_page(struct page *page, struct list_head *list, ClearPageCompound(head);

• split_page_owner(head, nr);

• split_page_owner(head, nr, 1);

Shouldn't this be 0, not 1? (new_order not new_nr).

/* See comment in __split_huge_page_tail() */ if (PageAnon(head)) { diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d77220615fd5..a9eead0e091a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3284,7 +3284,7 @@ void split_page(struct page *page, unsigned int order) for (i = 1; i < (1 << order); i++) set_page_refcounted(page + i);

• split_page_owner(page, 1 << order);

• split_page_owner(page, 1 << order, 1);

Ditto, 0.

} EXPORT_SYMBOL_GPL(split_page);

diff --git a/mm/page_owner.c b/mm/page_owner.c index b735a8eafcdb..2b7f7e9056dc 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -204,7 +204,7 @@ void __set_page_owner_migrate_reason(struct page *page, int reason) page_owner->last_migrate_reason = reason; } -void __split_page_owner(struct page *page, unsigned int nr) +void __split_page_owner(struct page *page, unsigned int nr, unsigned int new_order) { int i; struct page_ext *page_ext = lookup_page_ext(page); @@ -213,9 +213,9 @@ void __split_page_owner(struct page *page, unsigned int nr) if (unlikely(!page_ext)) return;

• for (i = 0; i < nr; i++) {

• for (i = 0; i < nr; i += (1 << new_order)) { page_owner = get_page_owner(page_ext);

• page_owner->order = 0;
  

• page_owner->order = new_order;
  

  page_ext = page_ext_next(page_ext); } }

On Wed, Nov 11, 2020 at 03:40:05PM -0500, Zi Yan wrote:

From: Zi Yan ziy@nvidia.com

It adds a new_order parameter to set new page order in page owner. It prepares for upcoming changes to support split huge page to any lower order.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/page_owner.h | 7 ++++--- mm/huge_memory.c | 2 +- mm/page_alloc.c | 2 +- mm/page_owner.c | 6 +++--- 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h index 3468794f83d2..215cbb159568 100644 --- a/include/linux/page_owner.h +++ b/include/linux/page_owner.h @@ -31,10 +31,11 @@ static inline void set_page_owner(struct page *page, __set_page_owner(page, order, gfp_mask); } -static inline void split_page_owner(struct page *page, unsigned int nr) +static inline void split_page_owner(struct page *page, unsigned int nr,

• 	unsigned int new_order)
  

{ if (static_branch_unlikely(&page_owner_inited))

• __split_page_owner(page, nr);
  

• __split_page_owner(page, nr, new_order);
  

} static inline void copy_page_owner(struct page *oldpage, struct page *newpage) { @@ -60,7 +61,7 @@ static inline void set_page_owner(struct page *page, { } static inline void split_page_owner(struct page *page,

• 	unsigned int order)
  

• 	unsigned int nr, unsigned int new_order)
  

With the addition of the new argument it's a bit hard to understand what the function is supposed to do. It seems like nr == page_order(page), is it right? Maybe we can pass old_order and new_order? Or just the page and the new order?

{ } static inline void copy_page_owner(struct page *oldpage, struct page *newpage) diff --git a/mm/huge_memory.c b/mm/huge_memory.c index f599f5b9bf7f..8b7d771ee962 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2459,7 +2459,7 @@ static void __split_huge_page(struct page *page, struct list_head *list, ClearPageCompound(head);

• split_page_owner(head, nr);

• split_page_owner(head, nr, 1);

/* See comment in __split_huge_page_tail() */ if (PageAnon(head)) { diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d77220615fd5..a9eead0e091a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3284,7 +3284,7 @@ void split_page(struct page *page, unsigned int order) for (i = 1; i < (1 << order); i++) set_page_refcounted(page + i);

• split_page_owner(page, 1 << order);

• split_page_owner(page, 1 << order, 1);

} EXPORT_SYMBOL_GPL(split_page); diff --git a/mm/page_owner.c b/mm/page_owner.c index b735a8eafcdb..2b7f7e9056dc 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -204,7 +204,7 @@ void __set_page_owner_migrate_reason(struct page *page, int reason) page_owner->last_migrate_reason = reason; } -void __split_page_owner(struct page *page, unsigned int nr) +void __split_page_owner(struct page *page, unsigned int nr, unsigned int new_order) { int i; struct page_ext *page_ext = lookup_page_ext(page); @@ -213,9 +213,9 @@ void __split_page_owner(struct page *page, unsigned int nr) if (unlikely(!page_ext)) return;

• for (i = 0; i < nr; i++) {

• for (i = 0; i < nr; i += (1 << new_order)) { page_owner = get_page_owner(page_ext);

• page_owner->order = 0;
  

• page_owner->order = new_order;
  

  page_ext = page_ext_next(page_ext);

I believe there cannot be any leftovers because nr is always a power of 2. Is it true? Converting nr argument to order (if it's possible) will make it obvious.

Other than that the patch looks good to me.

Thanks!

On Fri, Nov 13, 2020 at 08:08:58PM -0500, Zi Yan wrote:

On 13 Nov 2020, at 19:15, Roman Gushchin wrote:

...

On Wed, Nov 11, 2020 at 03:40:05PM -0500, Zi Yan wrote:

...

From: Zi Yan ziy@nvidia.com

It adds a new_order parameter to set new page order in page owner. It prepares for upcoming changes to support split huge page to any lower order.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/page_owner.h | 7 ++++--- mm/huge_memory.c | 2 +- mm/page_alloc.c | 2 +- mm/page_owner.c | 6 +++--- 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h index 3468794f83d2..215cbb159568 100644 --- a/include/linux/page_owner.h +++ b/include/linux/page_owner.h @@ -31,10 +31,11 @@ static inline void set_page_owner(struct page *page, __set_page_owner(page, order, gfp_mask); }

-static inline void split_page_owner(struct page *page, unsigned int nr) +static inline void split_page_owner(struct page *page, unsigned int nr,

• 	unsigned int new_order)
  

{ if (static_branch_unlikely(&page_owner_inited))

• __split_page_owner(page, nr);
  

• __split_page_owner(page, nr, new_order);
  

} static inline void copy_page_owner(struct page *oldpage, struct page *newpage) { @@ -60,7 +61,7 @@ static inline void set_page_owner(struct page *page, { } static inline void split_page_owner(struct page *page,

• 	unsigned int order)
  

• 	unsigned int nr, unsigned int new_order)
  

With the addition of the new argument it's a bit hard to understand what the function is supposed to do. It seems like nr == page_order(page), is it right? Maybe we can pass old_order and new_order? Or just the page and the new order?

Yeah, it is a bit confusing. Please see more below.

...

...

{ } static inline void copy_page_owner(struct page *oldpage, struct page *newpage) diff --git a/mm/huge_memory.c b/mm/huge_memory.c index f599f5b9bf7f..8b7d771ee962 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2459,7 +2459,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,

ClearPageCompound(head);

• split_page_owner(head, nr);

• split_page_owner(head, nr, 1);

  /* See comment in __split_huge_page_tail() */ if (PageAnon(head)) {

diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d77220615fd5..a9eead0e091a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3284,7 +3284,7 @@ void split_page(struct page *page, unsigned int order)

for (i = 1; i < (1 << order); i++) set_page_refcounted(page + i);

• split_page_owner(page, 1 << order);

• split_page_owner(page, 1 << order, 1);

} EXPORT_SYMBOL_GPL(split_page);

diff --git a/mm/page_owner.c b/mm/page_owner.c index b735a8eafcdb..2b7f7e9056dc 100644 --- a/mm/page_owner.c +++ b/mm/page_owner.c @@ -204,7 +204,7 @@ void __set_page_owner_migrate_reason(struct page *page, int reason) page_owner->last_migrate_reason = reason; }

-void __split_page_owner(struct page *page, unsigned int nr) +void __split_page_owner(struct page *page, unsigned int nr, unsigned int new_order) { int i; struct page_ext *page_ext = lookup_page_ext(page); @@ -213,9 +213,9 @@ void __split_page_owner(struct page *page, unsigned int nr) if (unlikely(!page_ext)) return;

• for (i = 0; i < nr; i++) {

• for (i = 0; i < nr; i += (1 << new_order)) { page_owner = get_page_owner(page_ext);

• page_owner->order = 0;
  

• page_owner->order = new_order;
  

  page_ext = page_ext_next(page_ext);

I believe there cannot be any leftovers because nr is always a power of 2. Is it true? Converting nr argument to order (if it's possible) will make it obvious.

Right. nr = thp_nr_pages(head), which is a power of 2. There would not be any leftover.

Matthew recently converted split_page_owner to take nr instead of order.[1] But I am not sure why, since it seems to me that two call sites (__split_huge_page in mm/huge_memory.c and split_page in mm/page_alloc.c) can pass the order information.

Yeah, I'm not sure why too. Maybe Matthew has some input here? You can also pass new_nr, but IMO orders look so much better here.

Thanks!

On 16 Nov 2020, at 11:25, Kirill A. Shutemov wrote:

On Wed, Nov 11, 2020 at 03:40:05PM -0500, Zi Yan wrote:

...

From: Zi Yan ziy@nvidia.com

It adds a new_order parameter to set new page order in page owner. It prepares for upcoming changes to support split huge page to any lower order.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/page_owner.h | 7 ++++--- mm/huge_memory.c | 2 +- mm/page_alloc.c | 2 +- mm/page_owner.c | 6 +++--- 4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h index 3468794f83d2..215cbb159568 100644 --- a/include/linux/page_owner.h +++ b/include/linux/page_owner.h @@ -31,10 +31,11 @@ static inline void set_page_owner(struct page *page, __set_page_owner(page, order, gfp_mask); }

-static inline void split_page_owner(struct page *page, unsigned int nr) +static inline void split_page_owner(struct page *page, unsigned int nr,

• 	unsigned int new_order)
  

{ if (static_branch_unlikely(&page_owner_inited))

• __split_page_owner(page, nr);
  

• __split_page_owner(page, nr, new_order);
  

Hm. Where do you correct __split_page_owner() declaration. I don't see it.

I missed it. Will add it. Thanks.

— Best Regards, Yan Zi

On 11/11/20 12:40 PM, Zi Yan wrote:

From: Zi Yan ziy@nvidia.com

To split a THP to any lower order pages, we need to reform THPs on subpages at given order and add page refcount based on the new page order. Also we need to reinitialize page_deferred_list after removing the page from the split_queue, otherwise a subsequent split will see list corruption when checking the page_deferred_list again.

It has many uses, like minimizing the number of pages after truncating a pagecache THP. For anonymous THPs, we can only split them to order-0 like before until we add support for any size anonymous THPs.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/huge_mm.h | 8 +++++ mm/huge_memory.c | 78 +++++++++++++++++++++++++++++------------ mm/swap.c | 1 - 3 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index 60a907a19f7d..9819cd9b4619 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -189,6 +189,8 @@ bool is_transparent_hugepage(struct page *page); bool can_split_huge_page(struct page *page, int *pextra_pins); int split_huge_page_to_list(struct page *page, struct list_head *list); +int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,

• unsigned int new_order);
  

  static inline int split_huge_page(struct page *page) { return split_huge_page_to_list(page, NULL);

@@ -396,6 +398,12 @@ split_huge_page_to_list(struct page *page, struct list_head *list) { return 0; } +static inline int +split_huge_page_to_order_to_list(struct page *page, struct list_head *list,

• unsigned int new_order)
  

+{

• return 0;

+} static inline int split_huge_page(struct page *page) { return 0; diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 8b7d771ee962..88f50da40c9b 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2327,11 +2327,14 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma, static void unmap_page(struct page *page) { enum ttu_flags ttu_flags = TTU_IGNORE_MLOCK | TTU_IGNORE_ACCESS |

• TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
  

• TTU_RMAP_LOCKED;
  

  bool unmap_success;

VM_BUG_ON_PAGE(!PageHead(page), page);

• if (thp_order(page) >= HPAGE_PMD_ORDER)

• ttu_flags |= TTU_SPLIT_HUGE_PMD;
  

• if (PageAnon(page)) ttu_flags |= TTU_SPLIT_FREEZE;

@@ -2339,21 +2342,22 @@ static void unmap_page(struct page *page) VM_BUG_ON_PAGE(!unmap_success, page); } -static void remap_page(struct page *page, unsigned int nr) +static void remap_page(struct page *page, unsigned int nr, unsigned int new_nr) { int i;

Use unsigned int i? Maybe a blank line here and the {}'s around if/else aren't needed.

• if (PageTransHuge(page)) {

• if (thp_nr_pages(page) == nr) { remove_migration_ptes(page, page, true); } else {

• for (i = 0; i < nr; i++)
  

• for (i = 0; i < nr; i += new_nr)
  remove_migration_ptes(page + i, page + i, true);
  

  } }

static void __split_huge_page_tail(struct page *head, int tail,

• struct lruvec *lruvec, struct list_head *list)
  

• struct lruvec *lruvec, struct list_head *list, unsigned int new_order)
  

  { struct page *page_tail = head + tail;
• unsigned long compound_head_flag = new_order ? (1L << PG_head) : 0;

VM_BUG_ON_PAGE(atomic_read(&page_tail->_mapcount) != -1, page_tail); @@ -2377,6 +2381,7 @@ static void __split_huge_page_tail(struct page *head, int tail, #ifdef CONFIG_64BIT (1L << PG_arch_2) | #endif

• 	 compound_head_flag |
   (1L << PG_dirty)));
  

/* ->mapping in first tail page is compound_mapcount */ @@ -2395,10 +2400,15 @@ static void __split_huge_page_tail(struct page *head, int tail, * which needs correct compound_head(). */ clear_compound_head(page_tail);

• if (new_order) {

• prep_compound_page(page_tail, new_order);
  

• thp_prep(page_tail);
  

• }

/* Finally unfreeze refcount. Additional reference from page cache. */

• page_ref_unfreeze(page_tail, 1 + (!PageAnon(head) ||

• 			  PageSwapCache(head)));
  

• page_ref_unfreeze(page_tail, 1 + ((!PageAnon(head) ||

• 			   PageSwapCache(head)) ?
  

• 				thp_nr_pages(page_tail) : 0));
  

if (page_is_young(head)) set_page_young(page_tail); @@ -2416,7 +2426,7 @@ static void __split_huge_page_tail(struct page *head, int tail, } static void __split_huge_page(struct page *page, struct list_head *list,

• pgoff_t end, unsigned long flags)
  

• pgoff_t end, unsigned long flags, unsigned int new_order)
  

  { struct page *head = compound_head(page); pg_data_t *pgdat = page_pgdat(head);

@@ -2424,12 +2434,13 @@ static void __split_huge_page(struct page *page, struct list_head *list, struct address_space *swap_cache = NULL; unsigned long offset = 0; unsigned int nr = thp_nr_pages(head);

• unsigned int new_nr = 1 << new_order; int i;

lruvec = mem_cgroup_page_lruvec(head, pgdat); /* complete memcg works before add pages to LRU */

• mem_cgroup_split_huge_fixup(head, 1);

• mem_cgroup_split_huge_fixup(head, new_nr);

if (PageAnon(head) && PageSwapCache(head)) { swp_entry_t entry = { .val = page_private(head) }; @@ -2439,14 +2450,14 @@ static void __split_huge_page(struct page *page, struct list_head *list, xa_lock(&swap_cache->i_pages); }

• for (i = nr - 1; i >= 1; i--) {

• __split_huge_page_tail(head, i, lruvec, list);
  

• for (i = nr - new_nr; i >= new_nr; i -= new_nr) {

• __split_huge_page_tail(head, i, lruvec, list, new_order);
  

  /* Some pages can be beyond i_size: drop them from page cache */ if (head[i].index >= end) { ClearPageDirty(head + i); __delete_from_page_cache(head + i, NULL); if (IS_ENABLED(CONFIG_SHMEM) && PageSwapBacked(head))

• 		shmem_uncharge(head->mapping->host, 1);
  

• 		shmem_uncharge(head->mapping->host, new_nr);
  put_page(head + i);
  

  } else if (!PageAnon(page)) { __xa_store(&head->mapping->i_pages, head[i].index,

@@ -2457,28 +2468,31 @@ static void __split_huge_page(struct page *page, struct list_head *list, } }

• ClearPageCompound(head);

• if (!new_order)

• ClearPageCompound(head);
  

• else

• set_compound_order(head, new_order);
  

• split_page_owner(head, nr, 1);

• split_page_owner(head, nr, new_nr);

This needs to be "new_order" instead of "new_nr".

/* See comment in __split_huge_page_tail() */ if (PageAnon(head)) { /* Additional pin to swap cache */ if (PageSwapCache(head)) {

• 	page_ref_add(head, 2);
  

• 	page_ref_add(head, 1 + new_nr);
  xa_unlock(&swap_cache->i_pages);
  

  } else { page_ref_inc(head); } } else { /* Additional pin to page cache */

• page_ref_add(head, 2);
  

• page_ref_add(head, 1 + new_nr);
  

  xa_unlock(&head->mapping->i_pages); }

spin_unlock_irqrestore(&pgdat->lru_lock, flags);

• remap_page(head, nr);

• remap_page(head, nr, new_nr);

if (PageSwapCache(head)) { swp_entry_t entry = { .val = page_private(head) }; @@ -2486,7 +2500,7 @@ static void __split_huge_page(struct page *page, struct list_head *list, split_swap_cluster(entry); }

• for (i = 0; i < nr; i++) {

• for (i = 0; i < nr; i += new_nr) { struct page *subpage = head + i; if (subpage == page) continue;

@@ -2620,21 +2634,39 @@ bool can_split_huge_page(struct page *page, int *pextra_pins)

• us.

*/ int split_huge_page_to_list(struct page *page, struct list_head *list) +{

• return split_huge_page_to_list_to_order(page, list, 0);

+}

+int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,

• 		     unsigned int new_order)
  

  { struct page *head = compound_head(page); struct pglist_data *pgdata = NODE_DATA(page_to_nid(head)); struct deferred_split *ds_queue = get_deferred_split_queue(head);

• XA_STATE(xas, &head->mapping->i_pages, head->index);

• /* reset xarray order to new order after split */
• XA_STATE_ORDER(xas, &head->mapping->i_pages, head->index, new_order); struct anon_vma *anon_vma = NULL; struct address_space *mapping = NULL; int count, mapcount, extra_pins, ret; unsigned long flags; pgoff_t end;

• VM_BUG_ON(thp_order(head) <= new_order); VM_BUG_ON_PAGE(is_huge_zero_page(head), head); VM_BUG_ON_PAGE(!PageLocked(head), head); VM_BUG_ON_PAGE(!PageCompound(head), head);

• if (new_order == 1) {

• WARN_ONCE(1, "Cannot split THP to order-1 (no order-1 THPs)");
  

• return -EINVAL;
  

• }
• if (PageAnon(head) && new_order) {

• WARN_ONCE(1, "Split anonymous THP to non-zero order not support");
  

• return -EINVAL;
  

• }
• if (PageWriteback(head)) return -EBUSY;

@@ -2720,18 +2752,18 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) if (!mapcount && page_ref_freeze(head, 1 + extra_pins)) { if (!list_empty(page_deferred_list(head))) { ds_queue->split_queue_len--;

• 	list_del(page_deferred_list(head));
  

• 	list_del_init(page_deferred_list(head));
  

  } spin_unlock(&ds_queue->split_queue_lock); if (mapping) { if (PageSwapBacked(head)) __dec_lruvec_page_state(head, NR_SHMEM_THPS);

• 	else
  

• 	else if (!new_order)
  	__mod_lruvec_page_state(head, NR_FILE_THPS,
  			-thp_nr_pages(head));
  

  }

• __split_huge_page(page, list, end, flags);
  

• __split_huge_page(page, list, end, flags, new_order);
  

  ret = 0; } else { if (IS_ENABLED(CONFIG_DEBUG_VM) && mapcount) {

@@ -2746,7 +2778,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list) fail: if (mapping) xas_unlock(&xas); spin_unlock_irqrestore(&pgdata->lru_lock, flags);

• remap_page(head, thp_nr_pages(head));
  

• remap_page(head, thp_nr_pages(head), 0);
  

Shouldn't this be "1" instead of zero? remap_page() takes new_nr.

ret = -EBUSY;

} diff --git a/mm/swap.c b/mm/swap.c index 14c3bac607a6..6c33e6165597 100644 --- a/mm/swap.c +++ b/mm/swap.c @@ -983,7 +983,6 @@ void lru_add_page_tail(struct page *page, struct page *page_tail, struct lruvec *lruvec, struct list_head *list) { VM_BUG_ON_PAGE(!PageHead(page), page);

• VM_BUG_ON_PAGE(PageCompound(page_tail), page); VM_BUG_ON_PAGE(PageLRU(page_tail), page); lockdep_assert_held(&lruvec_pgdat(lruvec)->lru_lock);

On Wed, Nov 11, 2020 at 03:40:06PM -0500, Zi Yan wrote:

From: Zi Yan ziy@nvidia.com

To split a THP to any lower order pages, we need to reform THPs on subpages at given order and add page refcount based on the new page order. Also we need to reinitialize page_deferred_list after removing the page from the split_queue, otherwise a subsequent split will see list corruption when checking the page_deferred_list again.

It has many uses, like minimizing the number of pages after truncating a pagecache THP. For anonymous THPs, we can only split them to order-0 like before until we add support for any size anonymous THPs.

Signed-off-by: Zi Yan ziy@nvidia.com

include/linux/huge_mm.h | 8 +++++ mm/huge_memory.c | 78 +++++++++++++++++++++++++++++------------ mm/swap.c | 1 - 3 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index 60a907a19f7d..9819cd9b4619 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -189,6 +189,8 @@ bool is_transparent_hugepage(struct page *page); bool can_split_huge_page(struct page *page, int *pextra_pins); int split_huge_page_to_list(struct page *page, struct list_head *list); +int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,

• unsigned int new_order);
  

static inline int split_huge_page(struct page *page) { return split_huge_page_to_list(page, NULL); @@ -396,6 +398,12 @@ split_huge_page_to_list(struct page *page, struct list_head *list) { return 0; } +static inline int +split_huge_page_to_order_to_list(struct page *page, struct list_head *list,

• unsigned int new_order)
  

It was int split_huge_page_to_list_to_order(struct page *page, struct list_head *list, unsigned int new_order); above.

+{

• return 0;

+} static inline int split_huge_page(struct page *page) { return 0; diff --git a/mm/huge_memory.c b/mm/huge_memory.c index 8b7d771ee962..88f50da40c9b 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -2327,11 +2327,14 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma, static void unmap_page(struct page *page) { enum ttu_flags ttu_flags = TTU_IGNORE_MLOCK | TTU_IGNORE_ACCESS |

• TTU_RMAP_LOCKED | TTU_SPLIT_HUGE_PMD;
  

• TTU_RMAP_LOCKED;
  

  bool unmap_success;

VM_BUG_ON_PAGE(!PageHead(page), page);

• if (thp_order(page) >= HPAGE_PMD_ORDER)

• ttu_flags |= TTU_SPLIT_HUGE_PMD;
  

• if (PageAnon(page)) ttu_flags |= TTU_SPLIT_FREEZE;

@@ -2339,21 +2342,22 @@ static void unmap_page(struct page *page) VM_BUG_ON_PAGE(!unmap_success, page); } -static void remap_page(struct page *page, unsigned int nr) +static void remap_page(struct page *page, unsigned int nr, unsigned int new_nr) { int i;

• if (PageTransHuge(page)) {

• if (thp_nr_pages(page) == nr) { remove_migration_ptes(page, page, true); } else {

• for (i = 0; i < nr; i++)
  

• for (i = 0; i < nr; i += new_nr)
  remove_migration_ptes(page + i, page + i, true);
  

  }

} static void __split_huge_page_tail(struct page *head, int tail,

• struct lruvec *lruvec, struct list_head *list)
  

• struct lruvec *lruvec, struct list_head *list, unsigned int new_order)
  

{ struct page *page_tail = head + tail;

• unsigned long compound_head_flag = new_order ? (1L << PG_head) : 0;

VM_BUG_ON_PAGE(atomic_read(&page_tail->_mapcount) != -1, page_tail); @@ -2377,6 +2381,7 @@ static void __split_huge_page_tail(struct page *head, int tail, #ifdef CONFIG_64BIT (1L << PG_arch_2) | #endif

• 	 compound_head_flag |
   (1L << PG_dirty)));
  

/* ->mapping in first tail page is compound_mapcount */ @@ -2395,10 +2400,15 @@ static void __split_huge_page_tail(struct page *head, int tail, * which needs correct compound_head(). */ clear_compound_head(page_tail);

• if (new_order) {

• prep_compound_page(page_tail, new_order);
  

• thp_prep(page_tail);
  

• }

/* Finally unfreeze refcount. Additional reference from page cache. */

• page_ref_unfreeze(page_tail, 1 + (!PageAnon(head) ||

• 			  PageSwapCache(head)));
  

• page_ref_unfreeze(page_tail, 1 + ((!PageAnon(head) ||

• 			   PageSwapCache(head)) ?
  

• 				thp_nr_pages(page_tail) : 0));
  

if (page_is_young(head)) set_page_young(page_tail); @@ -2416,7 +2426,7 @@ static void __split_huge_page_tail(struct page *head, int tail, } static void __split_huge_page(struct page *page, struct list_head *list,

• pgoff_t end, unsigned long flags)
  

• pgoff_t end, unsigned long flags, unsigned int new_order)
  

{ struct page *head = compound_head(page); pg_data_t *pgdat = page_pgdat(head); @@ -2424,12 +2434,13 @@ static void __split_huge_page(struct page *page, struct list_head *list, struct address_space *swap_cache = NULL; unsigned long offset = 0; unsigned int nr = thp_nr_pages(head);

• unsigned int new_nr = 1 << new_order; int i;

lruvec = mem_cgroup_page_lruvec(head, pgdat); /* complete memcg works before add pages to LRU */

• mem_cgroup_split_huge_fixup(head, 1);

• mem_cgroup_split_huge_fixup(head, new_nr);

if (PageAnon(head) && PageSwapCache(head)) { swp_entry_t entry = { .val = page_private(head) }; @@ -2439,14 +2450,14 @@ static void __split_huge_page(struct page *page, struct list_head *list, xa_lock(&swap_cache->i_pages); }

• for (i = nr - 1; i >= 1; i--) {

• __split_huge_page_tail(head, i, lruvec, list);
  

• for (i = nr - new_nr; i >= new_nr; i -= new_nr) {

• __split_huge_page_tail(head, i, lruvec, list, new_order);
  

  /* Some pages can be beyond i_size: drop them from page cache */ if (head[i].index >= end) { ClearPageDirty(head + i); __delete_from_page_cache(head + i, NULL); if (IS_ENABLED(CONFIG_SHMEM) && PageSwapBacked(head))

• 		shmem_uncharge(head->mapping->host, 1);
  

• 		shmem_uncharge(head->mapping->host, new_nr);
  put_page(head + i);
  

  } else if (!PageAnon(page)) { __xa_store(&head->mapping->i_pages, head[i].index,

@@ -2457,28 +2468,31 @@ static void __split_huge_page(struct page *page, struct list_head *list, } }

• ClearPageCompound(head);

• if (!new_order)

• ClearPageCompound(head);
  

• else

• set_compound_order(head, new_order);
  

• split_page_owner(head, nr, 1);

• split_page_owner(head, nr, new_nr);

/* See comment in __split_huge_page_tail() */ if (PageAnon(head)) { /* Additional pin to swap cache */ if (PageSwapCache(head)) {

• 	page_ref_add(head, 2);
  

• 	page_ref_add(head, 1 + new_nr);
  xa_unlock(&swap_cache->i_pages);
  

  } else { page_ref_inc(head); } } else { /* Additional pin to page cache */

• page_ref_add(head, 2);
  

• page_ref_add(head, 1 + new_nr);
  

  xa_unlock(&head->mapping->i_pages); }

spin_unlock_irqrestore(&pgdat->lru_lock, flags);

• remap_page(head, nr);

• remap_page(head, nr, new_nr);

if (PageSwapCache(head)) { swp_entry_t entry = { .val = page_private(head) }; @@ -2486,7 +2500,7 @@ static void __split_huge_page(struct page *page, struct list_head *list, split_swap_cluster(entry); }

• for (i = 0; i < nr; i++) {

• for (i = 0; i < nr; i += new_nr) { struct page *subpage = head + i; if (subpage == page) continue;

@@ -2620,21 +2634,39 @@ bool can_split_huge_page(struct page *page, int *pextra_pins)

• us.

*/ int split_huge_page_to_list(struct page *page, struct list_head *list) +{

• return split_huge_page_to_list_to_order(page, list, 0);

+}

+int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,

• 		     unsigned int new_order)
  

{ struct page *head = compound_head(page); struct pglist_data *pgdata = NODE_DATA(page_to_nid(head)); struct deferred_split *ds_queue = get_deferred_split_queue(head);

• XA_STATE(xas, &head->mapping->i_pages, head->index);

• /* reset xarray order to new order after split */
• XA_STATE_ORDER(xas, &head->mapping->i_pages, head->index, new_order); struct anon_vma *anon_vma = NULL; struct address_space *mapping = NULL; int count, mapcount, extra_pins, ret; unsigned long flags; pgoff_t end;

• VM_BUG_ON(thp_order(head) <= new_order); VM_BUG_ON_PAGE(is_huge_zero_page(head), head); VM_BUG_ON_PAGE(!PageLocked(head), head); VM_BUG_ON_PAGE(!PageCompound(head), head);

• if (new_order == 1) {

• WARN_ONCE(1, "Cannot split THP to order-1 (no order-1 THPs)");
  

• return -EINVAL;
  

• }
• if (PageAnon(head) && new_order) {

• WARN_ONCE(1, "Split anonymous THP to non-zero order not support");
  

• return -EINVAL;
  

• }

I'd convert those into VM_BUG_ON()'s. Unlikely they will be hit at arbitrary moments by random users.

Thanks!

On 12 Nov 2020, at 17:08, Ralph Campbell wrote:

On 11/11/20 12:40 PM, Zi Yan wrote:

...

From: Zi Yan ziy@nvidia.com

To minimize the number of pages after a truncation, when truncating a THP, we do not need to split it all the way down to order-0. The THP has at most three parts, the part before offset, the part to be truncated, the part left at the end. Use the non-zero minimum of them to decide what order we split the THP to.

Signed-off-by: Zi Yan ziy@nvidia.com

mm/truncate.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/mm/truncate.c b/mm/truncate.c index 20bd17538ec2..6d8e3c6115bc 100644 --- a/mm/truncate.c +++ b/mm/truncate.c @@ -237,7 +237,7 @@ int truncate_inode_page(struct address_space *mapping, struct page *page) bool truncate_inode_partial_page(struct page *page, loff_t start, loff_t end) { loff_t pos = page_offset(page);

• unsigned int offset, length;

• unsigned int offset, length, left, min_subpage_size = PAGE_SIZE;

Maybe use "remaining" instead of "left" since I think of the latter as the length of the left side (offset).

Sure. Will change the name.

...

if (pos < start) offset = start - pos; @@ -248,6 +248,7 @@ bool truncate_inode_partial_page(struct page *page, loff_t start, loff_t end) length = length - offset; else length = end + 1 - pos - offset;

• left = thp_size(page) - offset - length; wait_on_page_writeback(page); if (length == thp_size(page)) {

@@ -267,7 +268,24 @@ bool truncate_inode_partial_page(struct page *page, loff_t start, loff_t end) do_invalidatepage(page, offset, length); if (!PageTransHuge(page)) return true;

• return split_huge_page(page) == 0;

• /*

• * find the non-zero minimum of offset, length, and left and use it to
  

• * decide the new order of the page after split
  

• */
  

• if (offset && left)

• min_subpage_size = min_t(unsigned int,
  

• 			 min_t(unsigned int, offset, length),
  

• 			 left);
  

• else if (!offset)

• min_subpage_size = min_t(unsigned int, length, left);
  

• else /* !left */

• min_subpage_size = min_t(unsigned int, length, offset);
  

• min_subpage_size = max_t(unsigned int, PAGE_SIZE, min_subpage_size);
• return split_huge_page_to_list_to_order(page, NULL,

• 		ilog2(min_subpage_size/PAGE_SIZE)) == 0;
  

  }

What if "min_subpage_size" is 1/2 the THP but offset isn't aligned to 1/2? Splitting the page in half wouldn't result in a page that could be freed but maybe splitting to 1/4 would (assuming the THP is at least 8x PAGE_SIZE).

Is it possible? The whole THP is divided into three parts, offset, length, and remaining (renamed from left). If offset is not aligned to 1/2, it is either greater than 1/2 or smaller than 1/2. If it is the former, length and remaining will be smaller than 1/2, so min_subpage_size cannot be 1/2. If it is the latter, min_subpage_size cannot be 1/2 either. Because min_subpage_size is the smallest non-zero value of offset, length, and remaining. Let me know if I miss anything.

— Best Regards, Yan Zi