On 16 Nov 2020, at 11:06, Kirill A. Shutemov wrote:
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?
Sorry. Will fix it.
- ret = sscanf(input_buf, "%d,%lx,%lx", &pid, &vaddr_start, &vaddr_end);
Why hex without 0x prefix?
No particular reason. Let me add the 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.
I don’t need it. I just reuse the find_mm_struct function from mm/migrate.c.
- 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.
I forgot about this. I was trying to be smart to skip the rest of subpages if we split a THP. I will increase addr always by PAGE_SIZE to handle this situation.
/* 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.
If split_huge_page() succeeds, page_size(page) would be PAGE_SIZE and addr was increased by THP size - PAGE_SIZE above, so addr now should be at the end of the original THP.
Anyway, I will change the code to something like:
/* * always increase addr by PAGE_SIZE, since we could have a PTE page * table filled with PTE-mapped THPs, each of which is distinct. */ for (addr = vaddr_start; addr < vaddr_end; addr += PAGE_SIZE) { ...
if (!trylock_page(page)) continue;
split_huge_page(page);
unlock_page(page); put_page(page); } mmap_read_unlock(mm);
Thanks for reviewing the patch.
— Best Regards, Yan Zi