All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Mel Gorman <mgorman@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	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 v2 12/20] mm, hugetlb: remove vma_has_reserves()
Date: Thu, 22 Aug 2013 16:34:22 +0530	[thread overview]
Message-ID: <87mwoa0yx5.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20130822091747.GA22605@lge.com>

Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:

> On Thu, Aug 22, 2013 at 02:14:38PM +0530, Aneesh Kumar K.V wrote:
>> Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
>> 
>> > vma_has_reserves() can be substituted by using return value of
>> > vma_needs_reservation(). If chg returned by vma_needs_reservation()
>> > is 0, it means that vma has reserves. Otherwise, it means that vma don't
>> > have reserves and need a hugepage outside of reserve pool. This definition
>> > is perfectly same as vma_has_reserves(), so remove vma_has_reserves().
>> >
>> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>> 
>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> Thanks.
>
>> > @@ -580,8 +547,7 @@ static struct page *dequeue_huge_page_vma(struct hstate *h,
>> >  	 * have no page reserves. This check ensures that reservations are
>> >  	 * not "stolen". The child may still get SIGKILLed
>> >  	 */
>> > -	if (!vma_has_reserves(vma, chg) &&
>> > -			h->free_huge_pages - h->resv_huge_pages == 0)
>> > +	if (chg && h->free_huge_pages - h->resv_huge_pages == 0)
>> >  		return NULL;
>> >
>> >  	/* If reserves cannot be used, ensure enough pages are in the pool */
>> > @@ -600,7 +566,7 @@ retry_cpuset:
>> >  			if (page) {
>> >  				if (avoid_reserve)
>> >  					break;
>> > -				if (!vma_has_reserves(vma, chg))
>> > +				if (chg)
>> >  					break;
>> >
>> >  				SetPagePrivate(page);
>> 
>> Can you add a comment above both the place to explain why checking chg
>> is good enough ?
>
> Yes, I can. But it will be changed to use_reserve in patch 13 and it
> represent it's meaning perfectly. So commeting may be useless.

That should be ok, because having a comment in this patch helps in
understanding the patch better, even though you are removing that
later. 

-aneesh

--
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: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Mel Gorman <mgorman@suse.de>,
	Michal Hocko <mhocko@suse.cz>,
	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 v2 12/20] mm, hugetlb: remove vma_has_reserves()
Date: Thu, 22 Aug 2013 16:34:22 +0530	[thread overview]
Message-ID: <87mwoa0yx5.fsf@linux.vnet.ibm.com> (raw)
In-Reply-To: <20130822091747.GA22605@lge.com>

Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:

> On Thu, Aug 22, 2013 at 02:14:38PM +0530, Aneesh Kumar K.V wrote:
>> Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
>> 
>> > vma_has_reserves() can be substituted by using return value of
>> > vma_needs_reservation(). If chg returned by vma_needs_reservation()
>> > is 0, it means that vma has reserves. Otherwise, it means that vma don't
>> > have reserves and need a hugepage outside of reserve pool. This definition
>> > is perfectly same as vma_has_reserves(), so remove vma_has_reserves().
>> >
>> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>> 
>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> Thanks.
>
>> > @@ -580,8 +547,7 @@ static struct page *dequeue_huge_page_vma(struct hstate *h,
>> >  	 * have no page reserves. This check ensures that reservations are
>> >  	 * not "stolen". The child may still get SIGKILLed
>> >  	 */
>> > -	if (!vma_has_reserves(vma, chg) &&
>> > -			h->free_huge_pages - h->resv_huge_pages == 0)
>> > +	if (chg && h->free_huge_pages - h->resv_huge_pages == 0)
>> >  		return NULL;
>> >
>> >  	/* If reserves cannot be used, ensure enough pages are in the pool */
>> > @@ -600,7 +566,7 @@ retry_cpuset:
>> >  			if (page) {
>> >  				if (avoid_reserve)
>> >  					break;
>> > -				if (!vma_has_reserves(vma, chg))
>> > +				if (chg)
>> >  					break;
>> >
>> >  				SetPagePrivate(page);
>> 
>> Can you add a comment above both the place to explain why checking chg
>> is good enough ?
>
> Yes, I can. But it will be changed to use_reserve in patch 13 and it
> represent it's meaning perfectly. So commeting may be useless.

That should be ok, because having a comment in this patch helps in
understanding the patch better, even though you are removing that
later. 

-aneesh


  reply	other threads:[~2013-08-22 11:05 UTC|newest]

Thread overview: 139+ 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 ` 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-09  9:26   ` Joonsoo Kim
2013-08-12 13:20   ` Davidlohr Bueso
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-09  9:26   ` Joonsoo Kim
2013-08-12 13:21   ` Davidlohr Bueso
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-09  9:26   ` Joonsoo Kim
2013-08-21  9:28   ` Aneesh Kumar K.V
2013-08-21  9:28     ` Aneesh Kumar K.V
2013-08-22  6:50     ` Joonsoo Kim
2013-08-22  6:50       ` Joonsoo Kim
2013-08-22  7:08       ` Aneesh Kumar K.V
2013-08-22  7:08         ` Aneesh Kumar K.V
2013-08-22  7:47         ` Joonsoo Kim
2013-08-22  7:47           ` Joonsoo Kim
2013-08-26 13:01           ` Aneesh Kumar K.V
2013-08-26 13:01             ` Aneesh Kumar K.V
2013-08-27  7:40             ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-12 13:31   ` Davidlohr Bueso
2013-08-12 13:31     ` Davidlohr Bueso
2013-08-21  9:30   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-12 13:35   ` Davidlohr Bueso
2013-08-12 13:35     ` Davidlohr Bueso
2013-08-21  9:31   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-21  9:54   ` Aneesh Kumar K.V
2013-08-21  9:54     ` Aneesh Kumar K.V
2013-08-22  6:51     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-21  9:57   ` Aneesh Kumar K.V
2013-08-21  9:57     ` Aneesh Kumar K.V
2013-08-22  6:56     ` Joonsoo Kim
2013-08-22  6:56       ` Joonsoo Kim
2013-08-21 10:22   ` Aneesh Kumar K.V
2013-08-21 10:22     ` Aneesh Kumar K.V
2013-08-22  6:53     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-21  9:58   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-12 22:03   ` Davidlohr Bueso
2013-08-12 22:03     ` Davidlohr Bueso
2013-08-13  7:45     ` Joonsoo Kim
2013-08-13  7:45       ` Joonsoo Kim
2013-08-21 10:13   ` Aneesh Kumar K.V
2013-08-21 10:13     ` Aneesh Kumar K.V
2013-08-22  6:59     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-21 10:49   ` Aneesh Kumar K.V
2013-08-21 10:49     ` Aneesh Kumar K.V
2013-08-22  7:24     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-21 10:37   ` Aneesh Kumar K.V
2013-08-21 10:37     ` Aneesh Kumar K.V
2013-08-22  7:25     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-22  8:44   ` Aneesh Kumar K.V
2013-08-22  8:44     ` Aneesh Kumar K.V
2013-08-22  9:17     ` Joonsoo Kim
2013-08-22  9:17       ` Joonsoo Kim
2013-08-22 11:04       ` Aneesh Kumar K.V [this message]
2013-08-22 11:04         ` Aneesh Kumar K.V
2013-08-23  6:16         ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-26 13:09   ` Aneesh Kumar K.V
2013-08-26 13:09     ` Aneesh Kumar K.V
2013-08-27  7:57     ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-26 13:36   ` Aneesh Kumar K.V
2013-08-26 13:36     ` Aneesh Kumar K.V
2013-08-26 13:46     ` Aneesh Kumar K.V
2013-08-26 13:46       ` Aneesh Kumar K.V
2013-08-27  7:58       ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-26 13:38   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-26 13:44   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-26 14:09   ` Aneesh Kumar K.V
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-09  9:26   ` Joonsoo Kim
2013-08-26 14:12   ` Aneesh Kumar K.V
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-08-09  9:26   ` Joonsoo Kim
2013-09-04  8:44   ` 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-05  5:43       ` Joonsoo Kim
2013-09-16 12:09       ` David Gibson
2013-09-30  7:47         ` Joonsoo Kim
2013-09-30  7:47           ` Joonsoo Kim
2013-12-09 16:36           ` Davidlohr Bueso
2013-12-09 16:36             ` Davidlohr Bueso
2013-12-10  8:32             ` Joonsoo Kim
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-09  9:26   ` Joonsoo Kim
2013-08-14 23:22 ` [PATCH v2 00/20] " Andrew Morton
2013-08-14 23:22   ` Andrew Morton
2013-08-16 17:18   ` JoonSoo Kim
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=87mwoa0yx5.fsf@linux.vnet.ibm.com \
    --to=aneesh.kumar@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=davidlohr.bueso@hp.com \
    --cc=dhillf@gmail.com \
    --cc=hughd@google.com \
    --cc=iamjoonsoo.kim@lge.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.