Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754285AbbGWWJJ (ORCPT ); Thu, 23 Jul 2015 18:09:09 -0400 Received: from mail-pd0-f179.google.com ([209.85.192.179]:33192 "EHLO mail-pd0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753777AbbGWWJC (ORCPT ); Thu, 23 Jul 2015 18:09:02 -0400 Date: Thu, 23 Jul 2015 15:08:58 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Spencer Baugh cc: Andrew Morton , Naoya Horiguchi , Davidlohr Bueso , Mike Kravetz , Luiz Capitulino , "open list:MEMORY MANAGEMENT" , open list , Joern Engel , Spencer Baugh , Joern Engel Subject: Re: [PATCH] hugetlb: cond_resched for set_max_huge_pages and follow_hugetlb_page In-Reply-To: <1437688476-3399-1-git-send-email-sbaugh@catern.com> Message-ID: References: <1437688476-3399-1-git-send-email-sbaugh@catern.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1795 Lines: 48 On Thu, 23 Jul 2015, Spencer Baugh wrote: > From: Joern Engel > > ~150ms scheduler latency for both observed in the wild. > > Signed-off-by: Joern Engel > Signed-off-by: Spencer Baugh > --- > mm/hugetlb.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index a8c3087..2eb6919 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -1836,6 +1836,7 @@ static unsigned long set_max_huge_pages(struct hstate *h, unsigned long count, > ret = alloc_fresh_gigantic_page(h, nodes_allowed); > else > ret = alloc_fresh_huge_page(h, nodes_allowed); > + cond_resched(); > spin_lock(&hugetlb_lock); > if (!ret) > goto out; This is wrong, you'd want to do any cond_resched() before the page allocation to avoid racing with an update to h->nr_huge_pages or h->surplus_huge_pages while hugetlb_lock was dropped that would result in the page having been uselessly allocated. > @@ -3521,6 +3522,7 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, > spin_unlock(ptl); > ret = hugetlb_fault(mm, vma, vaddr, > (flags & FOLL_WRITE) ? FAULT_FLAG_WRITE : 0); > + cond_resched(); > if (!(ret & VM_FAULT_ERROR)) > continue; > This is almost certainly the wrong placement as well since it's inserted inside a conditional inside a while loop and there's no reason to hugetlb_fault(), schedule, and then check the return value. You need to insert your cond_resched()'s in legitimate places. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/