All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Mel Gorman <mgorman@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Hugh Dickins <hughd@google.com>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Wanpeng Li <liwanp@linux.vnet.ibm.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Hillf Danton <dhillf@gmail.com>
Subject: Re: [PATCH v3 13/14] mm, hugetlb: retry if failed to allocate and there is concurrent user
Date: Fri, 20 Dec 2013 13:47:09 +0900	[thread overview]
Message-ID: <20131220044709.GA1370@lge.com> (raw)
In-Reply-To: <1387506681.8363.55.camel@buesod1.americas.hpqcorp.net>

Hello, Davidlohr.

On Thu, Dec 19, 2013 at 06:31:21PM -0800, Davidlohr Bueso wrote:
> On Thu, 2013-12-19 at 17:02 -0800, Andrew Morton wrote:
> > On Wed, 18 Dec 2013 15:53:59 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > 
> > > If parallel fault occur, we can fail to allocate a hugepage,
> > > because many threads dequeue a hugepage to handle a fault of same address.
> > > This makes reserved pool shortage just for a little while and this cause
> > > faulting thread who can get hugepages to get a SIGBUS signal.
> > > 
> > > To solve this problem, we already have a nice solution, that is,
> > > a hugetlb_instantiation_mutex. This blocks other threads to dive into
> > > a fault handler. This solve the problem clearly, but it introduce
> > > performance degradation, because it serialize all fault handling.
> > > 
> > > Now, I try to remove a hugetlb_instantiation_mutex to get rid of
> > > performance degradation.
> > 
> > So the whole point of the patch is to improve performance, but the
> > changelog doesn't include any performance measurements!
> > 
> > Please, run some quantitative tests and include a nice summary of the
> > results in the changelog.
> 
> I was actually spending this afternoon testing these patches with Oracle
> (I haven't seen any issues so far) and unless Joonsoo already did so, I
> want to run these by the libhugetlb test cases - I got side tracked by
> futexes though :/

Really thanks for your time to test these patches.
I already did libhugetlbfs test cases and passed it.

> 
> Please do consider that performance wise I haven't seen much in
> particular. The thing is, I started dealing with this mutex once I
> noticed it as the #1 hot lock in Oracle DB starts, but then once the
> faults are done, it really goes away. So I wouldn't say that the mutex
> is a bottleneck except for the first few minutes.

What I want to be sure is for the first few minutes you mentioned.
If possible, let me know the result like as following link.
https://lkml.org/lkml/2013/7/12/428

Thanks in advance. :)

> > 
> > This is terribly important, because if the performance benefit is
> > infinitesimally small or negative, the patch goes into the bit bucket ;)
> 
> Well, this mutex is infinitesimally ugly and needs to die (as long as
> performance isn't hurt).

Yes, I agreed.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Davidlohr Bueso <davidlohr@hp.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Mel Gorman <mgorman@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Hugh Dickins <hughd@google.com>,
	Davidlohr Bueso <davidlohr.bueso@hp.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Wanpeng Li <liwanp@linux.vnet.ibm.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Hillf Danton <dhillf@gmail.com>
Subject: Re: [PATCH v3 13/14] mm, hugetlb: retry if failed to allocate and there is concurrent user
Date: Fri, 20 Dec 2013 13:47:09 +0900	[thread overview]
Message-ID: <20131220044709.GA1370@lge.com> (raw)
In-Reply-To: <1387506681.8363.55.camel@buesod1.americas.hpqcorp.net>

Hello, Davidlohr.

On Thu, Dec 19, 2013 at 06:31:21PM -0800, Davidlohr Bueso wrote:
> On Thu, 2013-12-19 at 17:02 -0800, Andrew Morton wrote:
> > On Wed, 18 Dec 2013 15:53:59 +0900 Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> > 
> > > If parallel fault occur, we can fail to allocate a hugepage,
> > > because many threads dequeue a hugepage to handle a fault of same address.
> > > This makes reserved pool shortage just for a little while and this cause
> > > faulting thread who can get hugepages to get a SIGBUS signal.
> > > 
> > > To solve this problem, we already have a nice solution, that is,
> > > a hugetlb_instantiation_mutex. This blocks other threads to dive into
> > > a fault handler. This solve the problem clearly, but it introduce
> > > performance degradation, because it serialize all fault handling.
> > > 
> > > Now, I try to remove a hugetlb_instantiation_mutex to get rid of
> > > performance degradation.
> > 
> > So the whole point of the patch is to improve performance, but the
> > changelog doesn't include any performance measurements!
> > 
> > Please, run some quantitative tests and include a nice summary of the
> > results in the changelog.
> 
> I was actually spending this afternoon testing these patches with Oracle
> (I haven't seen any issues so far) and unless Joonsoo already did so, I
> want to run these by the libhugetlb test cases - I got side tracked by
> futexes though :/

Really thanks for your time to test these patches.
I already did libhugetlbfs test cases and passed it.

> 
> Please do consider that performance wise I haven't seen much in
> particular. The thing is, I started dealing with this mutex once I
> noticed it as the #1 hot lock in Oracle DB starts, but then once the
> faults are done, it really goes away. So I wouldn't say that the mutex
> is a bottleneck except for the first few minutes.

What I want to be sure is for the first few minutes you mentioned.
If possible, let me know the result like as following link.
https://lkml.org/lkml/2013/7/12/428

Thanks in advance. :)

> > 
> > This is terribly important, because if the performance benefit is
> > infinitesimally small or negative, the patch goes into the bit bucket ;)
> 
> Well, this mutex is infinitesimally ugly and needs to die (as long as
> performance isn't hurt).

Yes, I agreed.

Thanks.

  reply	other threads:[~2013-12-20  4:47 UTC|newest]

Thread overview: 90+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-18  6:53 [PATCH v3 00/14] mm, hugetlb: remove a hugetlb_instantiation_mutex Joonsoo Kim
2013-12-18  6:53 ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 01/14] mm, hugetlb: unify region structure handling Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-21  9:04   ` David Gibson
2014-01-07  2:37   ` Davidlohr Bueso
2014-01-07  2:37     ` Davidlohr Bueso
2013-12-18  6:53 ` [PATCH v3 02/14] mm, hugetlb: region manipulation functions take resv_map rather list_head Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-21 13:43   ` David Gibson
2014-01-07  2:39   ` Davidlohr Bueso
2014-01-07  2:39     ` Davidlohr Bueso
2013-12-18  6:53 ` [PATCH v3 03/14] mm, hugetlb: protect region tracking via newly introduced resv_map lock Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-21 13:58   ` David Gibson
2013-12-23  1:05     ` Joonsoo Kim
2013-12-23  1:05       ` Joonsoo Kim
2013-12-24 12:00       ` David Gibson
2014-01-06  0:12         ` Joonsoo Kim
2014-01-06  0:12           ` Joonsoo Kim
2014-01-07  2:39   ` Davidlohr Bueso
2014-01-07  2:39     ` Davidlohr Bueso
2013-12-18  6:53 ` [PATCH v3 04/14] mm, hugetlb: remove resv_map_put() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 05/14] mm, hugetlb: make vma_resv_map() works for all mapping type Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 06/14] mm, hugetlb: remove vma_has_reserves() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 07/14] mm, hugetlb: mm, hugetlb: unify chg and avoid_reserve to use_reserve Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 08/14] mm, hugetlb: call vma_needs_reservation before entering alloc_huge_page() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 09/14] mm, hugetlb: remove a check for return value of alloc_huge_page() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 10/14] mm, hugetlb: move down outside_reserve check Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 11/14] mm, hugetlb: move up anon_vma_prepare() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 12/14] mm, hugetlb: clean-up error handling in hugetlb_cow() Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-18  6:53 ` [PATCH v3 13/14] mm, hugetlb: retry if failed to allocate and there is concurrent user Joonsoo Kim
2013-12-18  6:53   ` Joonsoo Kim
2013-12-20  1:02   ` Andrew Morton
2013-12-20  1:02     ` Andrew Morton
2013-12-20  1:58     ` Joonsoo Kim
2013-12-20  1:58       ` Joonsoo Kim
2013-12-20  2:15       ` Andrew Morton
2013-12-20  2:15         ` Andrew Morton
2013-12-20  5:00         ` Joonsoo Kim
2013-12-20  5:00           ` Joonsoo Kim
2013-12-20  2:31     ` Davidlohr Bueso
2013-12-20  2:31       ` Davidlohr Bueso
2013-12-20  4:47       ` Joonsoo Kim [this message]
2013-12-20  4:47         ` Joonsoo Kim
2013-12-20 14:01     ` Mel Gorman
2013-12-20 14:01       ` Mel Gorman
2013-12-21  6:48       ` Davidlohr Bueso
2013-12-21  6:48         ` Davidlohr Bueso
2013-12-23  0:44         ` Joonsoo Kim
2013-12-23  0:44           ` Joonsoo Kim
2013-12-23  2:11           ` Joonsoo Kim
2013-12-23  2:11             ` Joonsoo Kim
2014-01-03 19:55             ` Davidlohr Bueso
2014-01-03 19:55               ` Davidlohr Bueso
2014-01-06  0:19               ` Joonsoo Kim
2014-01-06  0:19                 ` Joonsoo Kim
2014-01-06 12:19                 ` Davidlohr Bueso
2014-01-06 12:19                   ` Davidlohr Bueso
2014-01-07  1:57                   ` Joonsoo Kim
2014-01-07  1:57                     ` Joonsoo Kim
2014-01-07  2:36                     ` Davidlohr Bueso
2014-01-07  2:36                       ` Davidlohr Bueso
2014-01-15  3:08                       ` David Rientjes
2014-01-15  3:08                         ` David Rientjes
2014-01-15  4:37                         ` Davidlohr Bueso
2014-01-15  4:37                           ` Davidlohr Bueso
2014-01-15  4:56                           ` Andrew Morton
2014-01-15  4:56                             ` Andrew Morton
2014-01-15 20:47                             ` Davidlohr Bueso
2014-01-15 20:47                               ` Davidlohr Bueso
2014-01-15 20:50                               ` Andrew Morton
2014-01-15 20:50                                 ` Andrew Morton
2013-12-18  6:54 ` [PATCH v3 14/14] mm, hugetlb: remove a hugetlb_instantiation_mutex Joonsoo Kim
2013-12-18  6:54   ` Joonsoo Kim
2014-03-31 16:27 ` [PATCH v3 00/14] " Dave Hansen
2014-03-31 16:27   ` Dave Hansen
2014-03-31 17:26   ` Davidlohr Bueso
2014-03-31 17:26     ` Davidlohr Bueso
2014-03-31 18:41     ` Dave Hansen
2014-03-31 18:41       ` Dave Hansen

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=20131220044709.GA1370@lge.com \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=davidlohr.bueso@hp.com \
    --cc=davidlohr@hp.com \
    --cc=dhillf@gmail.com \
    --cc=hughd@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liwanp@linux.vnet.ibm.com \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=riel@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.