linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: David Gibson <david@gibson.dropbear.id.au>
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>,
	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 v2 19/20] mm, hugetlb: retry if failed to allocate and there is concurrent user
Date: Mon, 30 Sep 2013 16:47:44 +0900	[thread overview]
Message-ID: <20130930074744.GA15351@lge.com> (raw)
In-Reply-To: <20130916120909.GA2706@voom.fritz.box>

On Mon, Sep 16, 2013 at 10:09:09PM +1000, David Gibson wrote:
> > > 
> > > > +		*do_dequeue = false;
> > > >  		spin_unlock(&hugetlb_lock);
> > > >  		page = alloc_buddy_huge_page(h, NUMA_NO_NODE);
> > > >  		if (!page) {
> > > 
> > > I think the counter also needs to be incremented in the case where we
> > > call alloc_buddy_huge_page() from alloc_huge_page().  Even though it's
> > > new, it gets added to the hugepage pool at this point and could still
> > > be a contended page for the last allocation, unless I'm missing
> > > something.
> > 
> > Your comment has reasonable point to me, but I have a different opinion.
> > 
> > As I already mentioned, the point is that we want to avoid the race
> > which kill the legitimate users of hugepages by out of resources.
> > I increase 'h->nr_dequeue_users' when the hugepage allocated by
> > administrator is dequeued. It is because what the hugepage I want to
> > protect from the race is the one allocated by administrator via
> > kernel param or /proc interface. Administrator may already know how many
> > hugepages are needed for their application so that he may set nr_hugepage
> > to reasonable value. I want to guarantee that these hugepages can be used
> > for his application without any race, since he assume that the application
> > would work fine with these hugepages.
> > 
> > To protect hugepages returned from alloc_buddy_huge_page() from the race
> > is different for me. Although it will be added to the hugepage pool, this
> > doesn't guarantee certain application's success more. If certain
> > application's success depends on the race of this new hugepage, it's death
> > by the race doesn't matter, since nobody assume that it works fine.
> 
> Hrm.  I still think this path should be included.  Although I'll agree
> that failing in this case is less bad.
> 
> However, it can still lead to a situation where with two processes or
> threads, faulting on exactly the same shared page we have one succeed
> and the other fail.  That's a strange behaviour and I think we want to
> avoid it in this case too.

Hello, David.

I don't think it is a strange behaviour. Similar situation can occur
even though we use the mutex. Hugepage allocation can be failed when
the first process try to allocate the hugepage while second process is blocked
by the mutex. And then, second process will go into the fault handler. And
at this time, it can succeed. So result is that we have one succeed and
the other fail.

It is slightly different from the case you mentioned, but I think that
effect for user is same. We cannot avoid this kind of race completely and
I think that avoiding the race for administrator managed hugepage pool is
good enough to use.

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>

  reply	other threads:[~2013-09-30  7:46 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-09  9:26 [PATCH v2 00/20] mm, hugetlb: remove a hugetlb_instantiation_mutex Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 01/20] mm, hugetlb: protect reserved pages when soft offlining a hugepage Joonsoo Kim
2013-08-12 13:20   ` Davidlohr Bueso
2013-08-09  9:26 ` [PATCH v2 02/20] mm, hugetlb: change variable name reservations to resv Joonsoo Kim
2013-08-12 13:21   ` Davidlohr Bueso
2013-08-09  9:26 ` [PATCH v2 03/20] mm, hugetlb: fix subpool accounting handling Joonsoo Kim
2013-08-21  9:28   ` Aneesh Kumar K.V
2013-08-22  6:50     ` Joonsoo Kim
2013-08-22  7:08       ` Aneesh Kumar K.V
2013-08-22  7:47         ` Joonsoo Kim
2013-08-26 13:01           ` Aneesh Kumar K.V
2013-08-27  7:40             ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 04/20] mm, hugetlb: remove useless check about mapping type Joonsoo Kim
2013-08-12 13:31   ` Davidlohr Bueso
2013-08-21  9:30   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 05/20] mm, hugetlb: grab a page_table_lock after page_cache_release Joonsoo Kim
2013-08-12 13:35   ` Davidlohr Bueso
2013-08-21  9:31   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 06/20] mm, hugetlb: return a reserved page to a reserved pool if failed Joonsoo Kim
2013-08-21  9:54   ` Aneesh Kumar K.V
2013-08-22  6:51     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 07/20] mm, hugetlb: unify region structure handling Joonsoo Kim
2013-08-21  9:57   ` Aneesh Kumar K.V
2013-08-22  6:56     ` Joonsoo Kim
2013-08-21 10:22   ` Aneesh Kumar K.V
2013-08-22  6:53     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 08/20] mm, hugetlb: region manipulation functions take resv_map rather list_head Joonsoo Kim
2013-08-21  9:58   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 09/20] mm, hugetlb: protect region tracking via newly introduced resv_map lock Joonsoo Kim
2013-08-12 22:03   ` Davidlohr Bueso
2013-08-13  7:45     ` Joonsoo Kim
2013-08-21 10:13   ` Aneesh Kumar K.V
2013-08-22  6:59     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 10/20] mm, hugetlb: remove resv_map_put() Joonsoo Kim
2013-08-21 10:49   ` Aneesh Kumar K.V
2013-08-22  7:24     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 11/20] mm, hugetlb: make vma_resv_map() works for all mapping type Joonsoo Kim
2013-08-21 10:37   ` Aneesh Kumar K.V
2013-08-22  7:25     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 12/20] mm, hugetlb: remove vma_has_reserves() Joonsoo Kim
2013-08-22  8:44   ` Aneesh Kumar K.V
2013-08-22  9:17     ` Joonsoo Kim
2013-08-22 11:04       ` Aneesh Kumar K.V
2013-08-23  6:16         ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 13/20] mm, hugetlb: mm, hugetlb: unify chg and avoid_reserve to use_reserve Joonsoo Kim
2013-08-26 13:09   ` Aneesh Kumar K.V
2013-08-27  7:57     ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 14/20] mm, hugetlb: call vma_needs_reservation before entering alloc_huge_page() Joonsoo Kim
2013-08-26 13:36   ` Aneesh Kumar K.V
2013-08-26 13:46     ` Aneesh Kumar K.V
2013-08-27  7:58       ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 15/20] mm, hugetlb: remove a check for return value of alloc_huge_page() Joonsoo Kim
2013-08-26 13:38   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 16/20] mm, hugetlb: move down outside_reserve check Joonsoo Kim
2013-08-26 13:44   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 17/20] mm, hugetlb: move up anon_vma_prepare() Joonsoo Kim
2013-08-26 14:09   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 18/20] mm, hugetlb: clean-up error handling in hugetlb_cow() Joonsoo Kim
2013-08-26 14:12   ` Aneesh Kumar K.V
2013-08-09  9:26 ` [PATCH v2 19/20] mm, hugetlb: retry if failed to allocate and there is concurrent user Joonsoo Kim
2013-09-04  8:44   ` Joonsoo Kim
2013-09-05  1:16     ` David Gibson
2013-09-05  1:15   ` David Gibson
2013-09-05  5:43     ` Joonsoo Kim
2013-09-16 12:09       ` David Gibson
2013-09-30  7:47         ` Joonsoo Kim [this message]
2013-12-09 16:36           ` Davidlohr Bueso
2013-12-10  8:32             ` Joonsoo Kim
2013-08-09  9:26 ` [PATCH v2 20/20] mm, hugetlb: remove a hugetlb_instantiation_mutex Joonsoo Kim
2013-08-14 23:22 ` [PATCH v2 00/20] " Andrew Morton
2013-08-16 17:18   ` JoonSoo Kim

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=20130930074744.GA15351@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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox