From: Mike Kravetz <mike.kravetz@oracle.com>
To: "Huang, Ying" <ying.huang@intel.com>,
Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrea Arcangeli <aarcange@redhat.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Andi Kleen <andi.kleen@intel.com>, Jan Kara <jack@suse.cz>,
Matthew Wilcox <mawilcox@microsoft.com>,
Hugh Dickins <hughd@google.com>, Minchan Kim <minchan@kernel.org>,
Shaohua Li <shli@fb.com>, Christopher Lameter <cl@linux.com>,
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
Punit Agrawal <punit.agrawal@arm.com>,
Anshuman Khandual <khandual@linux.vnet.ibm.com>,
David Rientjes <rientjes@google.com>,
Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH -V2 -mm] mm, hugetlbfs: Pass fault address to no page handler
Date: Thu, 17 May 2018 10:56:45 -0700 [thread overview]
Message-ID: <2eba3615-5144-f08d-169a-6cfd417d00b9@oracle.com> (raw)
In-Reply-To: <20180517083539.9242-1-ying.huang@intel.com>
On 05/17/2018 01:35 AM, Huang, Ying wrote:
> From: Huang Ying <ying.huang@intel.com>
>
> This is to take better advantage of general huge page clearing
> optimization (c79b57e462b5d, "mm: hugetlb: clear target sub-page last
> when clearing huge page") for hugetlbfs. In the general optimization
> patch, the sub-page to access will be cleared last to avoid the cache
> lines of to access sub-page to be evicted when clearing other
> sub-pages. This works better if we have the address of the sub-page
> to access, that is, the fault address inside the huge page. So the
> hugetlbfs no page fault handler is changed to pass that information.
> This will benefit workloads which don't access the begin of the
> hugetlbfs huge page after the page fault under heavy cache contention
> for shared last level cache.
>
> The patch is a generic optimization which should benefit quite some
> workloads, not for a specific use case. To demonstrate the performance
> benefit of the patch, we tested it with vm-scalability run on
> hugetlbfs.
>
> With this patch, the throughput increases ~28.1% in vm-scalability
> anon-w-seq test case with 88 processes on a 2 socket Xeon E5 2699 v4
> system (44 cores, 88 threads). The test case creates 88 processes,
> each process mmaps a big anonymous memory area with MAP_HUGETLB and
> writes to it from the end to the begin. For each process, other
> processes could be seen as other workload which generates heavy cache
> pressure. At the same time, the cache miss rate reduced from ~36.3%
> to ~25.6%, the IPC (instruction per cycle) increased from 0.3 to 0.37,
> and the time spent in user space is reduced ~19.3%.
>
Agree with Michal that commit message looks better.
I went through updated patch with haddr naming so,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
still applies.
--
Mike Kravetz
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Andi Kleen <andi.kleen@intel.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Matthew Wilcox <mawilcox@microsoft.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Shaohua Li <shli@fb.com>
> Cc: Christopher Lameter <cl@linux.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> Acked-by: David Rientjes <rientjes@google.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> ---
> mm/hugetlb.c | 42 +++++++++++++++++++++---------------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 129088710510..4f0682cb9414 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3686,6 +3686,7 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> struct page *page;
> pte_t new_pte;
> spinlock_t *ptl;
> + unsigned long haddr = address & huge_page_mask(h);
>
> /*
> * Currently, we are forced to kill the process in the event the
> @@ -3716,7 +3717,7 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> u32 hash;
> struct vm_fault vmf = {
> .vma = vma,
> - .address = address,
> + .address = haddr,
> .flags = flags,
> /*
> * Hard to debug if it ends up being
> @@ -3733,14 +3734,14 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> * fault to make calling code simpler.
> */
> hash = hugetlb_fault_mutex_hash(h, mm, vma, mapping,
> - idx, address);
> + idx, haddr);
> mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> ret = handle_userfault(&vmf, VM_UFFD_MISSING);
> mutex_lock(&hugetlb_fault_mutex_table[hash]);
> goto out;
> }
>
> - page = alloc_huge_page(vma, address, 0);
> + page = alloc_huge_page(vma, haddr, 0);
> if (IS_ERR(page)) {
> ret = PTR_ERR(page);
> if (ret == -ENOMEM)
> @@ -3789,12 +3790,12 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> * the spinlock.
> */
> if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
> - if (vma_needs_reservation(h, vma, address) < 0) {
> + if (vma_needs_reservation(h, vma, haddr) < 0) {
> ret = VM_FAULT_OOM;
> goto backout_unlocked;
> }
> /* Just decrements count, does not deallocate */
> - vma_end_reservation(h, vma, address);
> + vma_end_reservation(h, vma, haddr);
> }
>
> ptl = huge_pte_lock(h, mm, ptep);
> @@ -3808,17 +3809,17 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
>
> if (anon_rmap) {
> ClearPagePrivate(page);
> - hugepage_add_new_anon_rmap(page, vma, address);
> + hugepage_add_new_anon_rmap(page, vma, haddr);
> } else
> page_dup_rmap(page, true);
> new_pte = make_huge_pte(vma, page, ((vma->vm_flags & VM_WRITE)
> && (vma->vm_flags & VM_SHARED)));
> - set_huge_pte_at(mm, address, ptep, new_pte);
> + set_huge_pte_at(mm, haddr, ptep, new_pte);
>
> hugetlb_count_add(pages_per_huge_page(h), mm);
> if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
> /* Optimization, do the COW without a second fault */
> - ret = hugetlb_cow(mm, vma, address, ptep, page, ptl);
> + ret = hugetlb_cow(mm, vma, haddr, ptep, page, ptl);
> }
>
> spin_unlock(ptl);
> @@ -3830,7 +3831,7 @@ static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
> spin_unlock(ptl);
> backout_unlocked:
> unlock_page(page);
> - restore_reserve_on_error(h, vma, address, page);
> + restore_reserve_on_error(h, vma, haddr, page);
> put_page(page);
> goto out;
> }
> @@ -3883,10 +3884,9 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> struct hstate *h = hstate_vma(vma);
> struct address_space *mapping;
> int need_wait_lock = 0;
> + unsigned long haddr = address & huge_page_mask(h);
>
> - address &= huge_page_mask(h);
> -
> - ptep = huge_pte_offset(mm, address, huge_page_size(h));
> + ptep = huge_pte_offset(mm, haddr, huge_page_size(h));
> if (ptep) {
> entry = huge_ptep_get(ptep);
> if (unlikely(is_hugetlb_entry_migration(entry))) {
> @@ -3896,20 +3896,20 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> return VM_FAULT_HWPOISON_LARGE |
> VM_FAULT_SET_HINDEX(hstate_index(h));
> } else {
> - ptep = huge_pte_alloc(mm, address, huge_page_size(h));
> + ptep = huge_pte_alloc(mm, haddr, huge_page_size(h));
> if (!ptep)
> return VM_FAULT_OOM;
> }
>
> mapping = vma->vm_file->f_mapping;
> - idx = vma_hugecache_offset(h, vma, address);
> + idx = vma_hugecache_offset(h, vma, haddr);
>
> /*
> * Serialize hugepage allocation and instantiation, so that we don't
> * get spurious allocation failures if two CPUs race to instantiate
> * the same page in the page cache.
> */
> - hash = hugetlb_fault_mutex_hash(h, mm, vma, mapping, idx, address);
> + hash = hugetlb_fault_mutex_hash(h, mm, vma, mapping, idx, haddr);
> mutex_lock(&hugetlb_fault_mutex_table[hash]);
>
> entry = huge_ptep_get(ptep);
> @@ -3939,16 +3939,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> * consumed.
> */
> if ((flags & FAULT_FLAG_WRITE) && !huge_pte_write(entry)) {
> - if (vma_needs_reservation(h, vma, address) < 0) {
> + if (vma_needs_reservation(h, vma, haddr) < 0) {
> ret = VM_FAULT_OOM;
> goto out_mutex;
> }
> /* Just decrements count, does not deallocate */
> - vma_end_reservation(h, vma, address);
> + vma_end_reservation(h, vma, haddr);
>
> if (!(vma->vm_flags & VM_MAYSHARE))
> pagecache_page = hugetlbfs_pagecache_page(h,
> - vma, address);
> + vma, haddr);
> }
>
> ptl = huge_pte_lock(h, mm, ptep);
> @@ -3973,16 +3973,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>
> if (flags & FAULT_FLAG_WRITE) {
> if (!huge_pte_write(entry)) {
> - ret = hugetlb_cow(mm, vma, address, ptep,
> + ret = hugetlb_cow(mm, vma, haddr, ptep,
> pagecache_page, ptl);
> goto out_put_page;
> }
> entry = huge_pte_mkdirty(entry);
> }
> entry = pte_mkyoung(entry);
> - if (huge_ptep_set_access_flags(vma, address, ptep, entry,
> + if (huge_ptep_set_access_flags(vma, haddr, ptep, entry,
> flags & FAULT_FLAG_WRITE))
> - update_mmu_cache(vma, address, ptep);
> + update_mmu_cache(vma, haddr, ptep);
> out_put_page:
> if (page != pagecache_page)
> unlock_page(page);
>
next prev parent reply other threads:[~2018-05-17 17:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-17 8:35 [PATCH -V2 -mm] mm, hugetlbfs: Pass fault address to no page handler Huang, Ying
2018-05-17 9:15 ` Michal Hocko
2018-05-18 0:33 ` Huang, Ying
2018-05-17 17:56 ` Mike Kravetz [this message]
2018-05-18 0:33 ` Huang, Ying
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2eba3615-5144-f08d-169a-6cfd417d00b9@oracle.com \
--to=mike.kravetz@oracle.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=andi.kleen@intel.com \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=cl@linux.com \
--cc=hughd@google.com \
--cc=jack@suse.cz \
--cc=khandual@linux.vnet.ibm.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mawilcox@microsoft.com \
--cc=mhocko@suse.com \
--cc=minchan@kernel.org \
--cc=punit.agrawal@arm.com \
--cc=rientjes@google.com \
--cc=shli@fb.com \
--cc=ying.huang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).