From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 392AAC433DB for ; Mon, 22 Mar 2021 16:29:58 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id AD83761923 for ; Mon, 22 Mar 2021 16:29:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AD83761923 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 149636B00F2; Mon, 22 Mar 2021 12:29:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F9BB6B00F4; Mon, 22 Mar 2021 12:29:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EB64C6B00F5; Mon, 22 Mar 2021 12:29:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0104.hostedemail.com [216.40.44.104]) by kanga.kvack.org (Postfix) with ESMTP id CA5476B00F2 for ; Mon, 22 Mar 2021 12:29:56 -0400 (EDT) Received: from smtpin11.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 8227D8E5A for ; Mon, 22 Mar 2021 16:29:56 +0000 (UTC) X-FDA: 77948046792.11.40494E1 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) by imf03.hostedemail.com (Postfix) with ESMTP id 9AF0EC0042EE for ; Mon, 22 Mar 2021 16:29:52 +0000 (UTC) Received: by mail-ed1-f42.google.com with SMTP id b16so20129075eds.7 for ; Mon, 22 Mar 2021 09:29:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+yQvc43S8r83HTfxyOEbHMJvrQICL6Aq5HxThzdm6Ks=; b=a2GjEUZ1xP95PzLsG5fmHkO4GpwMpY7HUMMd4ayLoTo9cG/yg09LhITO4tawX3Gh2O S7N+ryWZy5GouSEmtD1mb8SThKNuqs8Z9yN3ngdtTwDlYUEwij8RGRgLq1b5TWRGEUd1 lpEatp3TPzJo9OY97sCLpWhy9nfDF53L/h2iUXWniqZhnx142PstGYIRivRSbYwGo8Kg r9rWKcxnIt5xc3nKSVK8Z7oVtV74FsmAfOQ+zP3tJthYvxIRZt7w6HZ5+SZiVyVUS3XS fhbD029+04jfjdSQ9ATm5PtDSDdmIvSM8qmjNX3LIHLyZKFk8FWZuN6z7jV3s0P8r+kv T80Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+yQvc43S8r83HTfxyOEbHMJvrQICL6Aq5HxThzdm6Ks=; b=audEQN9RryqPYvRDgkwYxdIWmFMLxMQu4WnHjJ6REWPso7HkcXIiUQssxd3xphQQgA iCLQivkicO2YxEniCPWRmRD3zxzz52Wo78PmX7Ll/R5LxyAqjFD2NXaPADKhXu7+ogEV cZvtm1AUtNREKZdiZa1LnRJXEnlDykqI9hBHTEVxEB2INYHtMy+WforKNlMb8Qa6S6Kg EvcJENFPNATPdE9s4ToNbFPJDj2qO6lX8j302q0hglBePq39lbawQRsYAbz0Az3kJQAw R5I0D1vPOxd9FOyk6MNnqEGk41a8GJVz5hDMm4NhRpC4CI3O0JnV8mFIHnwnQH3IPyFO 4vaA== X-Gm-Message-State: AOAM533pwbS2QckXkstThhd6d9b2Ig//13ANf9ZP6beGVOjGsdZVOeHz 5kXRHUMCDZspIawmjjnzXls15zIZkBbG9DQ97gw= X-Google-Smtp-Source: ABdhPJw/98e78zuT8+5qNEOuBX8v/LXRegZluDS1c2KWIdAv/JnGkF+H8KcV3lYGkdIQsLlPNh9FOV8aUahW5xm10/M= X-Received: by 2002:a05:6402:518d:: with SMTP id q13mr363560edd.313.1616430590270; Mon, 22 Mar 2021 09:29:50 -0700 (PDT) MIME-Version: 1.0 References: <20210319005219.13595-1-zi.yan@sent.com> <176A4DF5-B5B9-47E2-9B9D-CD82989AC466@nvidia.com> In-Reply-To: <176A4DF5-B5B9-47E2-9B9D-CD82989AC466@nvidia.com> From: Yang Shi Date: Mon, 22 Mar 2021 09:29:38 -0700 Message-ID: Subject: Re: [PATCH v5 1/2] mm: huge_memory: a new debugfs interface for splitting THP tests. To: Zi Yan Cc: Linux MM , Linux Kernel Mailing List , linux-kselftest@vger.kernel.org, "Kirill A . Shutemov" , Andrew Morton , Shuah Khan , John Hubbard , Sandipan Das , David Hildenbrand , Mika Penttila Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: cternu69cfkqpb9njtqr7atphm4j8sgd X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 9AF0EC0042EE Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf03; identity=mailfrom; envelope-from=""; helo=mail-ed1-f42.google.com; client-ip=209.85.208.42 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616430592-993144 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, Mar 21, 2021 at 7:11 PM Zi Yan wrote: > > On 19 Mar 2021, at 19:37, Yang Shi wrote: > > > On Thu, Mar 18, 2021 at 5:52 PM Zi Yan wrote: > >> > >> From: Zi Yan > >> > >> We did not have a direct user interface of splitting the compound page > >> backing a THP and there is no need unless we want to expose the THP > >> implementation details to users. Make /split_huge_pages accept > >> a new command to do that. > >> > >> By writing ",," to > >> /split_huge_pages, THPs within the given virtual address range > >> from the process with the given pid are split. It is used to test > >> split_huge_page function. In addition, a selftest program is added to > >> tools/testing/selftests/vm to utilize the interface by splitting > >> PMD THPs and PTE-mapped THPs. > >> > >> This does not change the old behavior, i.e., writing 1 to the interface > >> to split all THPs in the system. > >> > >> Changelog: > >> > >> From v5: > >> 1. Skipped special VMAs and other fixes. (suggested by Yang Shi) > > > > Looks good to me. Reviewed-by: Yang Shi > > > > Some nits below: > > > >> > >> From v4: > >> 1. Fixed the error code return issue, spotted by kernel test robot > >> . > >> > >> From v3: > >> 1. Factored out split huge pages in the given pid code to a separate > >> function. > >> 2. Added the missing put_page for not split pages. > >> 3. pr_debug -> pr_info, make reading results simpler. > >> > >> From v2: > >> 1. Reused existing /split_huge_pages interface. (suggested by > >> Yang Shi) > >> > >> From v1: > >> 1. Removed unnecessary calling to vma_migratable, spotted by kernel test > >> robot . > >> 2. Dropped the use of find_mm_struct and code it directly, since there > >> is no need for the permission check in that function and the function > >> is only available when migration is on. > >> 3. Added some comments in the selftest program to clarify how PTE-mapped > >> THPs are formed. > >> > >> Signed-off-by: Zi Yan > >> --- > >> mm/huge_memory.c | 143 +++++++- > >> tools/testing/selftests/vm/.gitignore | 1 + > >> tools/testing/selftests/vm/Makefile | 1 + > >> .../selftests/vm/split_huge_page_test.c | 318 ++++++++++++++++++ > >> 4 files changed, 456 insertions(+), 7 deletions(-) > >> 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 bff92dea5ab3..9bf9bc489228 100644 > >> --- a/mm/huge_memory.c > >> +++ b/mm/huge_memory.c > >> @@ -7,6 +7,7 @@ > >> > >> #include > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -2922,16 +2923,14 @@ static struct shrinker deferred_split_shrinker = { > >> }; > >> > >> #ifdef CONFIG_DEBUG_FS > >> -static int split_huge_pages_set(void *data, u64 val) > >> +static void split_huge_pages_all(void) > >> { > >> struct zone *zone; > >> struct page *page; > >> unsigned long pfn, max_zone_pfn; > >> unsigned long total = 0, split = 0; > >> > >> - if (val != 1) > >> - return -EINVAL; > >> - > >> + pr_info("Split all THPs\n"); > >> for_each_populated_zone(zone) { > >> max_zone_pfn = zone_end_pfn(zone); > >> for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) { > >> @@ -2959,11 +2958,141 @@ static int split_huge_pages_set(void *data, u64 val) > >> } > >> > >> pr_info("%lu of %lu THP split\n", split, total); > >> +} > >> > >> - return 0; > >> +static int split_huge_pages_pid(int pid, unsigned long vaddr_start, > >> + unsigned long vaddr_end) > >> +{ > >> + int ret = 0; > >> + struct task_struct *task; > >> + struct mm_struct *mm; > >> + unsigned long total = 0, split = 0; > >> + unsigned long addr; > >> + > >> + vaddr_start &= PAGE_MASK; > >> + vaddr_end &= PAGE_MASK; > >> + > >> + /* Find the task_struct from pid */ > >> + rcu_read_lock(); > >> + task = find_task_by_vpid(pid); > >> + if (!task) { > >> + rcu_read_unlock(); > >> + ret = -ESRCH; > >> + goto out; > >> + } > >> + get_task_struct(task); > >> + rcu_read_unlock(); > >> + > >> + /* Find the mm_struct */ > >> + mm = get_task_mm(task); > >> + put_task_struct(task); > >> + > >> + if (!mm) { > >> + ret = -EINVAL; > >> + goto out; > >> + } > >> + > >> + pr_info("Split huge pages in pid: %d, vaddr: [0x%lx - 0x%lx]\n", > >> + pid, vaddr_start, vaddr_end); > >> + > >> + mmap_read_lock(mm); > >> + /* > >> + * 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) { > >> + struct vm_area_struct *vma = find_vma(mm, addr); > >> + unsigned int follflags; > >> + struct page *page; > >> + > >> + if (!vma || addr < vma->vm_start) > >> + break; > >> + > >> + /* skip special VMA and hugetlb VMA */ > >> + if (vma_is_special_huge(vma) || is_vm_hugetlb_page(vma)) { > > > > VM_IO vma should be skipped as well. And you may extract this into a helper? > > Sure. Any name suggestion? :) How's about not_suitable_for_split() or suitable_for_split()? > > > >> + addr = vma->vm_end; > >> + continue; > >> + } > >> + > >> + /* FOLL_DUMP to ignore special (like zero) pages */ > >> + follflags = FOLL_GET | FOLL_DUMP; > >> + page = follow_page(vma, addr, follflags); > >> + > >> + if (IS_ERR(page)) > >> + continue; > >> + if (!page) > >> + continue; > >> + > >> + if (!is_transparent_hugepage(page)) > >> + goto next; > >> + > >> + total++; > >> + if (!can_split_huge_page(compound_head(page), NULL)) > >> + goto next; > >> + > >> + if (!trylock_page(page)) > >> + goto next; > >> + > >> + if (!split_huge_page(page)) > >> + split++; > >> + > >> + unlock_page(page); > >> +next: > >> + put_page(page); > >> + } > >> + mmap_read_unlock(mm); > >> + mmput(mm); > >> + > >> + pr_info("%lu of %lu THP split\n", split, total); > >> + > >> +out: > >> + return ret; > >> } > >> -DEFINE_DEBUGFS_ATTRIBUTE(split_huge_pages_fops, NULL, split_huge_pages_set, > >> - "%llu\n"); > >> + > >> +static ssize_t split_huge_pages_write(struct file *file, const char __user *buf, > >> + size_t count, loff_t *ppops) > >> +{ > >> + static DEFINE_MUTEX(split_debug_mutex); > >> + ssize_t ret; > >> + char input_buf[80]; /* hold pid, start_vaddr, end_vaddr */ > > > > Why not move buf len macro in the following patch to this patch? Then > > you don't have to change the length again. > > Sure. > > Thanks for the comments. > > -- > Best Regards, > Yan Zi