From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-ob0-f174.google.com (mail-ob0-f174.google.com [209.85.214.174])
	by kanga.kvack.org (Postfix) with ESMTP id DD8536B0038
	for <linux-mm@kvack.org>; Mon, 11 May 2015 07:18:00 -0400 (EDT)
Received: by obblk2 with SMTP id lk2so97197738obb.0
        for <linux-mm@kvack.org>; Mon, 11 May 2015 04:18:00 -0700 (PDT)
Received: from aserp1040.oracle.com (aserp1040.oracle.com. [141.146.126.69])
        by mx.google.com with ESMTPS id xt18si6973488oeb.90.2015.05.11.04.18.00
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Mon, 11 May 2015 04:18:00 -0700 (PDT)
Date: Mon, 11 May 2015 14:17:48 +0300
From: Dan Carpenter <dan.carpenter@oracle.com>
Subject: re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Message-ID: <20150511111748.GA20660@mwanda>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: n-horiguchi@ah.jp.nec.com
Cc: linux-mm@kvack.org

Hello Naoya Horiguchi,

The patch c8721bbbdd36: "mm: memory-hotplug: enable memory hotplug to
handle hugepage" from Sep 11, 2013, leads to the following static
checker warning:

	mm/hugetlb.c:1203 dissolve_free_huge_pages()
	warn: potential right shift more than type allows '9,18,64'

mm/hugetlb.c
  1189  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
  1190  {
  1191          unsigned int order = 8 * sizeof(void *);
                                     ^^^^^^^^^^^^^^^^^^
Let's say order is 64.

  1192          unsigned long pfn;
  1193          struct hstate *h;
  1194  
  1195          if (!hugepages_supported())
  1196                  return;
  1197  
  1198          /* Set scan step to minimum hugepage size */
  1199          for_each_hstate(h)
  1200                  if (order > huge_page_order(h))
  1201                          order = huge_page_order(h);
  1202          VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
  1203          for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
                                                            ^^^^^^^^^^
1 << 64 is undefined but let's say it's zero because that's normal for
GCC.  This is an endless loop.

  1204                  dissolve_free_huge_page(pfn_to_page(pfn));
  1205  }

regards,
dan carpenter

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-oi0-f52.google.com (mail-oi0-f52.google.com [209.85.218.52])
	by kanga.kvack.org (Postfix) with ESMTP id 48A4C6B0038
	for <linux-mm@kvack.org>; Mon, 11 May 2015 07:29:34 -0400 (EDT)
Received: by oiko83 with SMTP id o83so101701465oik.1
        for <linux-mm@kvack.org>; Mon, 11 May 2015 04:29:34 -0700 (PDT)
Received: from aserp1040.oracle.com (aserp1040.oracle.com. [141.146.126.69])
        by mx.google.com with ESMTPS id bj4si6991370oec.64.2015.05.11.04.29.33
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Mon, 11 May 2015 04:29:33 -0700 (PDT)
Date: Mon, 11 May 2015 14:29:24 +0300
From: Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Message-ID: <20150511112924.GM16501@mwanda>
References: <20150511111748.GA20660@mwanda>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150511111748.GA20660@mwanda>
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: n-horiguchi@ah.jp.nec.com
Cc: linux-mm@kvack.org

On Mon, May 11, 2015 at 02:17:48PM +0300, Dan Carpenter wrote:
> Hello Naoya Horiguchi,
> 
> The patch c8721bbbdd36: "mm: memory-hotplug: enable memory hotplug to
> handle hugepage" from Sep 11, 2013, leads to the following static
> checker warning:
> 
> 	mm/hugetlb.c:1203 dissolve_free_huge_pages()
> 	warn: potential right shift more than type allows '9,18,64'
> 
> mm/hugetlb.c
>   1189  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
>   1190  {
>   1191          unsigned int order = 8 * sizeof(void *);
>                                      ^^^^^^^^^^^^^^^^^^
> Let's say order is 64.

Actually, the 64 here is just chosen to be an impossibly high number
isn't it?  It's a bit complicated to understand that at first glance.

regards,
dan carpenter

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-oi0-f41.google.com (mail-oi0-f41.google.com [209.85.218.41])
	by kanga.kvack.org (Postfix) with ESMTP id 0F5896B0038
	for <linux-mm@kvack.org>; Mon, 11 May 2015 19:55:30 -0400 (EDT)
Received: by oiko83 with SMTP id o83so117831203oik.1
        for <linux-mm@kvack.org>; Mon, 11 May 2015 16:55:29 -0700 (PDT)
Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp. [210.143.35.51])
        by mx.google.com with ESMTPS id l6si3256786oif.135.2015.05.11.16.55.28
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Mon, 11 May 2015 16:55:29 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Date: Mon, 11 May 2015 23:54:44 +0000
Message-ID: <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
In-Reply-To: <20150511111748.GA20660@mwanda>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <81DA28103FBCCF45BE8B9A549816B929@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

On Mon, May 11, 2015 at 02:17:48PM +0300, Dan Carpenter wrote:
> Hello Naoya Horiguchi,
>=20
> The patch c8721bbbdd36: "mm: memory-hotplug: enable memory hotplug to
> handle hugepage" from Sep 11, 2013, leads to the following static
> checker warning:
>=20
> 	mm/hugetlb.c:1203 dissolve_free_huge_pages()
> 	warn: potential right shift more than type allows '9,18,64'
>=20
> mm/hugetlb.c
>   1189  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned l=
ong end_pfn)
>   1190  {
>   1191          unsigned int order =3D 8 * sizeof(void *);
>                                      ^^^^^^^^^^^^^^^^^^
> Let's say order is 64.

Hi Dan, thank you for reporting.

order is supposed to be capped by running each hstates and finding the
minimum hugepage order as done in below code, and I intended that this
initialization gives potential maximum. I guess that keeping this to 64
doesn't solve the above warning, so we use 8 * sizeof(void *) - 1 or 63 ?
I don't test on 32-bit system, so not sure that this code can be used
by 32-bit system, but considering such case, keeping sizeof(void *)
seems better.

>=20
>   1192          unsigned long pfn;
>   1193          struct hstate *h;
>   1194 =20
>   1195          if (!hugepages_supported())
>   1196                  return;
>   1197 =20
>   1198          /* Set scan step to minimum hugepage size */
>   1199          for_each_hstate(h)
>   1200                  if (order > huge_page_order(h))
>   1201                          order =3D huge_page_order(h);
>   1202          VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
>   1203          for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << orde=
r)
>                                                             ^^^^^^^^^^
> 1 << 64 is undefined but let's say it's zero because that's normal for
> GCC.  This is an endless loop.

That never happens if hstates is properly initialized, but we had better
avoid the potential risk.

How about the following patch?

Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Tue, 12 May 2015 08:17:10 +0900
Subject: [PATCH] mm/hugetlb: decrement initial value of order in
 dissolve_free_huge_pages

Currently the initial value of order in dissolve_free_huge_page is 64 or 32=
,
which leads to the following warning in static checker:

  mm/hugetlb.c:1203 dissolve_free_huge_pages()
  warn: potential right shift more than type allows '9,18,64'

This is a potential risk of infinite loop, because 1 << order (=3D=3D 0) is=
 used
in for-loop like this:

  for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
      ...

So this patch simply avoids the risk by decrementing the initial value.

Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle h=
ugepage")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/hugetlb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c41b2a0ee273..74abfb44e4d0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct page *page=
)
  */
 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_p=
fn)
 {
-	unsigned int order =3D 8 * sizeof(void *);
+	/* Initialized to "high enough" value which is capped later */
+	unsigned int order =3D 8 * sizeof(void *) - 1;
 	unsigned long pfn;
 	struct hstate *h;
=20
--=20
2.1.0

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-pa0-f53.google.com (mail-pa0-f53.google.com [209.85.220.53])
	by kanga.kvack.org (Postfix) with ESMTP id 5ACA06B0038
	for <linux-mm@kvack.org>; Mon, 11 May 2015 19:56:54 -0400 (EDT)
Received: by pacyx8 with SMTP id yx8so122325123pac.1
        for <linux-mm@kvack.org>; Mon, 11 May 2015 16:56:54 -0700 (PDT)
Received: from tyo202.gate.nec.co.jp (TYO202.gate.nec.co.jp. [210.143.35.52])
        by mx.google.com with ESMTPS id v3si19868315pdh.200.2015.05.11.16.56.51
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Mon, 11 May 2015 16:56:53 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Date: Mon, 11 May 2015 23:56:25 +0000
Message-ID: <20150511235624.GB8513@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda> <20150511112924.GM16501@mwanda>
In-Reply-To: <20150511112924.GM16501@mwanda>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <788CA7358F03184496347376EF5DF4B7@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>

On Mon, May 11, 2015 at 02:29:24PM +0300, Dan Carpenter wrote:
> On Mon, May 11, 2015 at 02:17:48PM +0300, Dan Carpenter wrote:
> > Hello Naoya Horiguchi,
> >=20
> > The patch c8721bbbdd36: "mm: memory-hotplug: enable memory hotplug to
> > handle hugepage" from Sep 11, 2013, leads to the following static
> > checker warning:
> >=20
> > 	mm/hugetlb.c:1203 dissolve_free_huge_pages()
> > 	warn: potential right shift more than type allows '9,18,64'
> >=20
> > mm/hugetlb.c
> >   1189  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned=
 long end_pfn)
> >   1190  {
> >   1191          unsigned int order =3D 8 * sizeof(void *);
> >                                      ^^^^^^^^^^^^^^^^^^
> > Let's say order is 64.
>=20
> Actually, the 64 here is just chosen to be an impossibly high number
> isn't it?

Right.

>  It's a bit complicated to understand that at first glance.

OK, so I added oneline comment in the patch in another email.

Thanks,
Naoya Horiguchi=

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-qk0-f171.google.com (mail-qk0-f171.google.com [209.85.220.171])
	by kanga.kvack.org (Postfix) with ESMTP id 146596B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 04:43:54 -0400 (EDT)
Received: by qkgy4 with SMTP id y4so158909qkg.2
        for <linux-mm@kvack.org>; Tue, 12 May 2015 01:43:53 -0700 (PDT)
Received: from userp1040.oracle.com (userp1040.oracle.com. [156.151.31.81])
        by mx.google.com with ESMTPS id e145si7661294qhc.95.2015.05.12.01.43.52
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Tue, 12 May 2015 01:43:52 -0700 (PDT)
Date: Tue, 12 May 2015 11:43:39 +0300
From: Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Message-ID: <20150512084339.GN16501@mwanda>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

On Mon, May 11, 2015 at 11:54:44PM +0000, Naoya Horiguchi wrote:
> @@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct page *page)
>   */
>  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
>  {
> -	unsigned int order = 8 * sizeof(void *);
> +	/* Initialized to "high enough" value which is capped later */
> +	unsigned int order = 8 * sizeof(void *) - 1;

Why not use UINT_MAX?  It's more clear that it's not valid that way.
Otherwise doing a complicated calculation it makes it seem like we will
use the variable.

regards,
dan carpenter

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-oi0-f47.google.com (mail-oi0-f47.google.com [209.85.218.47])
	by kanga.kvack.org (Postfix) with ESMTP id BC4E06B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 05:05:51 -0400 (EDT)
Received: by oign205 with SMTP id n205so594736oig.2
        for <linux-mm@kvack.org>; Tue, 12 May 2015 02:05:51 -0700 (PDT)
Received: from tyo202.gate.nec.co.jp (TYO202.gate.nec.co.jp. [210.143.35.52])
        by mx.google.com with ESMTPS id p3si8478040oev.93.2015.05.12.02.05.50
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Tue, 12 May 2015 02:05:51 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Date: Tue, 12 May 2015 09:04:55 +0000
Message-ID: <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
In-Reply-To: <20150512084339.GN16501@mwanda>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <AE91DAC3236D3B4199F8A385A413789A@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

On Tue, May 12, 2015 at 11:43:39AM +0300, Dan Carpenter wrote:
> On Mon, May 11, 2015 at 11:54:44PM +0000, Naoya Horiguchi wrote:
> > @@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct page *=
page)
> >   */
> >  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long e=
nd_pfn)
> >  {
> > -	unsigned int order =3D 8 * sizeof(void *);
> > +	/* Initialized to "high enough" value which is capped later */
> > +	unsigned int order =3D 8 * sizeof(void *) - 1;
>=20
> Why not use UINT_MAX?  It's more clear that it's not valid that way.

It's OK if code checker doesn't show "too much right shift" warning.
With UINT_MAX, inserting VM_BUG_ON(order =3D=3D UINT_MAX) after for_each_hs=
tate
loop might be safer (1 << UINT_MAX is clearly wrong.)

> Otherwise doing a complicated calculation it makes it seem like we will
> use the variable.

OK.

Is the below OK for you?
---
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 271e4432734c..804437505a84 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1188,7 +1188,7 @@ static void dissolve_free_huge_page(struct page *page=
)
  */
 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_p=
fn)
 {
-	unsigned int order =3D 8 * sizeof(void *);
+	unsigned int order =3D UINT_MAX;
 	unsigned long pfn;
 	struct hstate *h;
=20
@@ -1200,6 +1200,7 @@ void dissolve_free_huge_pages(unsigned long start_pfn=
, unsigned long end_pfn)
 		if (order > huge_page_order(h))
 			order =3D huge_page_order(h);
 	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
+	VM_BUG_ON(order =3D=3D UINT_MAX);
 	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
 		dissolve_free_huge_page(pfn_to_page(pfn));
 }=

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-yh0-f46.google.com (mail-yh0-f46.google.com [209.85.213.46])
	by kanga.kvack.org (Postfix) with ESMTP id E33C66B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 05:14:02 -0400 (EDT)
Received: by yhrr66 with SMTP id r66so332696yhr.3
        for <linux-mm@kvack.org>; Tue, 12 May 2015 02:14:02 -0700 (PDT)
Received: from aserp1040.oracle.com (aserp1040.oracle.com. [141.146.126.69])
        by mx.google.com with ESMTPS id o6si8107710yke.67.2015.05.12.02.14.01
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Tue, 12 May 2015 02:14:01 -0700 (PDT)
Date: Tue, 12 May 2015 12:13:49 +0300
From: Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Message-ID: <20150512091349.GO16501@mwanda>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
 <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

On Tue, May 12, 2015 at 02:04:55AM -0700, Naoya Horiguchi wrote:
> On Tue, May 12, 2015 at 11:43:39AM +0300, Dan Carpenter wrote:
> > On Mon, May 11, 2015 at 11:54:44PM +0000, Naoya Horiguchi wrote:
> > > @@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct page *page)
> > >   */
> > >  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
> > >  {
> > > -	unsigned int order = 8 * sizeof(void *);
> > > +	/* Initialized to "high enough" value which is capped later */
> > > +	unsigned int order = 8 * sizeof(void *) - 1;
> > 
> > Why not use UINT_MAX?  It's more clear that it's not valid that way.
> 
> It's OK if code checker doesn't show "too much right shift" warning.

It's a comlicated question to answer but with the new VM_BUG_ON() then
it won't warn.

regards,
dan carpenter

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-ob0-f171.google.com (mail-ob0-f171.google.com [209.85.214.171])
	by kanga.kvack.org (Postfix) with ESMTP id 4D2366B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 05:17:23 -0400 (EDT)
Received: by obfe9 with SMTP id e9so756890obf.1
        for <linux-mm@kvack.org>; Tue, 12 May 2015 02:17:23 -0700 (PDT)
Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp. [210.143.35.51])
        by mx.google.com with ESMTPS id o185si8503664oib.141.2015.05.12.02.17.22
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Tue, 12 May 2015 02:17:22 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: Re: mm: memory-hotplug: enable memory hotplug to handle hugepage
Date: Tue, 12 May 2015 09:16:40 +0000
Message-ID: <20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
 <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512091349.GO16501@mwanda>
In-Reply-To: <20150512091349.GO16501@mwanda>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <45D026327805704DB7F48242F06E9852@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

On Tue, May 12, 2015 at 12:13:49PM +0300, Dan Carpenter wrote:
> On Tue, May 12, 2015 at 02:04:55AM -0700, Naoya Horiguchi wrote:
> > On Tue, May 12, 2015 at 11:43:39AM +0300, Dan Carpenter wrote:
> > > On Mon, May 11, 2015 at 11:54:44PM +0000, Naoya Horiguchi wrote:
> > > > @@ -1086,7 +1086,8 @@ static void dissolve_free_huge_page(struct pa=
ge *page)
> > > >   */
> > > >  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned lo=
ng end_pfn)
> > > >  {
> > > > -	unsigned int order =3D 8 * sizeof(void *);
> > > > +	/* Initialized to "high enough" value which is capped later */
> > > > +	unsigned int order =3D 8 * sizeof(void *) - 1;
> > >=20
> > > Why not use UINT_MAX?  It's more clear that it's not valid that way.
> >=20
> > It's OK if code checker doesn't show "too much right shift" warning.
>=20
> It's a comlicated question to answer but with the new VM_BUG_ON() then
> it won't warn.

Good, so I'll repost the patch soon later with revised description.

Thanks,
Naoya=

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-pd0-f179.google.com (mail-pd0-f179.google.com [209.85.192.179])
	by kanga.kvack.org (Postfix) with ESMTP id 510916B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 05:21:22 -0400 (EDT)
Received: by pdbqd1 with SMTP id qd1so1890340pdb.2
        for <linux-mm@kvack.org>; Tue, 12 May 2015 02:21:22 -0700 (PDT)
Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp. [210.143.35.51])
        by mx.google.com with ESMTPS id bv5si21690756pdb.125.2015.05.12.02.21.20
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Tue, 12 May 2015 02:21:21 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: [PATCH] mm/hugetlb: initialize order with UINT_MAX in
 dissolve_free_huge_pages()
Date: Tue, 12 May 2015 09:20:35 +0000
Message-ID: <20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
 <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512091349.GO16501@mwanda>
 <20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
In-Reply-To: <20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <E5CAA6C325FD96449FA0FB4F6189A7FD@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>

Currently the initial value of order in dissolve_free_huge_page is 64 or 32=
,
which leads to the following warning in static checker:

  mm/hugetlb.c:1203 dissolve_free_huge_pages()
  warn: potential right shift more than type allows '9,18,64'

This is a potential risk of infinite loop, because 1 << order (=3D=3D 0) is=
 used
in for-loop like this:

  for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
      ...

So this patch simply avoids the risk by initializing with UINT_MAX.

Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle h=
ugepage")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/hugetlb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 271e4432734c..804437505a84 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1188,7 +1188,7 @@ static void dissolve_free_huge_page(struct page *page=
)
  */
 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_p=
fn)
 {
-	unsigned int order =3D 8 * sizeof(void *);
+	unsigned int order =3D UINT_MAX;
 	unsigned long pfn;
 	struct hstate *h;
=20
@@ -1200,6 +1200,7 @@ void dissolve_free_huge_pages(unsigned long start_pfn=
, unsigned long end_pfn)
 		if (order > huge_page_order(h))
 			order =3D huge_page_order(h);
 	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
+	VM_BUG_ON(order =3D=3D UINT_MAX);
 	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
 		dissolve_free_huge_page(pfn_to_page(pfn));
 }
--=20
2.1.0

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-pa0-f51.google.com (mail-pa0-f51.google.com [209.85.220.51])
	by kanga.kvack.org (Postfix) with ESMTP id 095246B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 19:15:14 -0400 (EDT)
Received: by pacyx8 with SMTP id yx8so29565674pac.1
        for <linux-mm@kvack.org>; Tue, 12 May 2015 16:15:13 -0700 (PDT)
Received: from mail.linuxfoundation.org (mail.linuxfoundation.org. [140.211.169.12])
        by mx.google.com with ESMTPS id da1si24493439pad.9.2015.05.12.16.15.12
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Tue, 12 May 2015 16:15:13 -0700 (PDT)
Date: Tue, 12 May 2015 16:15:11 -0700
From: Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/hugetlb: initialize order with UINT_MAX in
 dissolve_free_huge_pages()
Message-Id: <20150512161511.7967c400cae6c1d693b61d57@linux-foundation.org>
In-Reply-To: <20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
	<20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
	<20150512084339.GN16501@mwanda>
	<20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
	<20150512091349.GO16501@mwanda>
	<20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
	<20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>

On Tue, 12 May 2015 09:20:35 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> Currently the initial value of order in dissolve_free_huge_page is 64 or 32,
> which leads to the following warning in static checker:
> 
>   mm/hugetlb.c:1203 dissolve_free_huge_pages()
>   warn: potential right shift more than type allows '9,18,64'
> 
> This is a potential risk of infinite loop, because 1 << order (== 0) is used
> in for-loop like this:
> 
>   for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
>       ...
> 
> So this patch simply avoids the risk by initializing with UINT_MAX.
> 
> ..
>
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1188,7 +1188,7 @@ static void dissolve_free_huge_page(struct page *page)
>   */
>  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
>  {
> -	unsigned int order = 8 * sizeof(void *);
> +	unsigned int order = UINT_MAX;
>  	unsigned long pfn;
>  	struct hstate *h;
>  
> @@ -1200,6 +1200,7 @@ void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_pfn)
>  		if (order > huge_page_order(h))
>  			order = huge_page_order(h);
>  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> +	VM_BUG_ON(order == UINT_MAX);
>  	for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
>  		dissolve_free_huge_page(pfn_to_page(pfn));

Do we need to calculate this each time?  Can it be done in
hugetlb_init_hstates(), save the result in a global?

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-ie0-f181.google.com (mail-ie0-f181.google.com [209.85.223.181])
	by kanga.kvack.org (Postfix) with ESMTP id 4155F6B0038
	for <linux-mm@kvack.org>; Tue, 12 May 2015 21:45:25 -0400 (EDT)
Received: by iebpz10 with SMTP id pz10so19108948ieb.2
        for <linux-mm@kvack.org>; Tue, 12 May 2015 18:45:25 -0700 (PDT)
Received: from tyo201.gate.nec.co.jp (TYO201.gate.nec.co.jp. [210.143.35.51])
        by mx.google.com with ESMTPS id q18si14049008ico.33.2015.05.12.18.45.24
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Tue, 12 May 2015 18:45:24 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: Re: [PATCH] mm/hugetlb: initialize order with UINT_MAX in
 dissolve_free_huge_pages()
Date: Wed, 13 May 2015 01:44:22 +0000
Message-ID: <20150513014418.GB14599@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
 <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512091349.GO16501@mwanda>
 <20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512161511.7967c400cae6c1d693b61d57@linux-foundation.org>
In-Reply-To: <20150512161511.7967c400cae6c1d693b61d57@linux-foundation.org>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <11DD24838ACFC74087128A6C8855AC18@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>

On Tue, May 12, 2015 at 04:15:11PM -0700, Andrew Morton wrote:
> On Tue, 12 May 2015 09:20:35 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec=
.com> wrote:
>=20
> > Currently the initial value of order in dissolve_free_huge_page is 64 o=
r 32,
> > which leads to the following warning in static checker:
> >=20
> >   mm/hugetlb.c:1203 dissolve_free_huge_pages()
> >   warn: potential right shift more than type allows '9,18,64'
> >=20
> > This is a potential risk of infinite loop, because 1 << order (=3D=3D 0=
) is used
> > in for-loop like this:
> >=20
> >   for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
> >       ...
> >=20
> > So this patch simply avoids the risk by initializing with UINT_MAX.
> >=20
> > ..
> >
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1188,7 +1188,7 @@ static void dissolve_free_huge_page(struct page *=
page)
> >   */
> >  void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long e=
nd_pfn)
> >  {
> > -	unsigned int order =3D 8 * sizeof(void *);
> > +	unsigned int order =3D UINT_MAX;
> >  	unsigned long pfn;
> >  	struct hstate *h;
> > =20
> > @@ -1200,6 +1200,7 @@ void dissolve_free_huge_pages(unsigned long start=
_pfn, unsigned long end_pfn)
> >  		if (order > huge_page_order(h))
> >  			order =3D huge_page_order(h);
> >  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> > +	VM_BUG_ON(order =3D=3D UINT_MAX);
> >  	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
> >  		dissolve_free_huge_page(pfn_to_page(pfn));
>=20
> Do we need to calculate this each time?  Can it be done in
> hugetlb_init_hstates(), save the result in a global?

Yes, it should work. How about the following?
This adds 4bytes to .data due to a new global variable, but reduces 47 byte=
s
.text size of code reduces, so it's a win in total.

   text    data     bss     dec     hex filename                        =20
  28313     469   84236  113018   1b97a mm/hugetlb.o (above patch)
  28266     473   84236  112975   1b94f mm/hugetlb.o (below patch)

---
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 271e4432734c..fecb8bbfe11e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -40,6 +40,7 @@ int hugepages_treat_as_movable;
 int hugetlb_max_hstate __read_mostly;
 unsigned int default_hstate_idx;
 struct hstate hstates[HUGE_MAX_HSTATE];
+unsigned int minimum_order __read_mostly;
=20
 __initdata LIST_HEAD(huge_boot_pages);
=20
@@ -1188,19 +1189,13 @@ static void dissolve_free_huge_page(struct page *pa=
ge)
  */
 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_p=
fn)
 {
-	unsigned int order =3D 8 * sizeof(void *);
 	unsigned long pfn;
-	struct hstate *h;
=20
 	if (!hugepages_supported())
 		return;
=20
-	/* Set scan step to minimum hugepage size */
-	for_each_hstate(h)
-		if (order > huge_page_order(h))
-			order =3D huge_page_order(h);
-	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
-	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
+	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << minimum_order));
+	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << minimum_order)
 		dissolve_free_huge_page(pfn_to_page(pfn));
 }
=20
@@ -1626,11 +1621,16 @@ static void __init hugetlb_init_hstates(void)
 {
 	struct hstate *h;
=20
+	minimum_order =3D UINT_MAX;
 	for_each_hstate(h) {
+		if (minimum_order > huge_page_order(h))
+			minimum_order =3D huge_page_order(h);
+
 		/* oversize hugepages were init'ed in early boot */
 		if (!hstate_is_gigantic(h))
 			hugetlb_hstate_alloc_pages(h);
 	}
+	VM_BUG_ON(minimum_order =3D=3D UINT_MAX);
 }
=20
 static char * __init memfmt(char *buf, unsigned long n)
--=20
2.1.0

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-pa0-f52.google.com (mail-pa0-f52.google.com [209.85.220.52])
	by kanga.kvack.org (Postfix) with ESMTP id C37B56B0038
	for <linux-mm@kvack.org>; Wed, 13 May 2015 16:55:58 -0400 (EDT)
Received: by pabsx10 with SMTP id sx10so61661533pab.3
        for <linux-mm@kvack.org>; Wed, 13 May 2015 13:55:58 -0700 (PDT)
Received: from mail.linuxfoundation.org (mail.linuxfoundation.org. [140.211.169.12])
        by mx.google.com with ESMTPS id bx10si28769940pdb.154.2015.05.13.13.55.57
        for <linux-mm@kvack.org>
        (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
        Wed, 13 May 2015 13:55:57 -0700 (PDT)
Date: Wed, 13 May 2015 13:55:56 -0700
From: Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/hugetlb: initialize order with UINT_MAX in
 dissolve_free_huge_pages()
Message-Id: <20150513135556.5d21cd52810f87460eb1f2a1@linux-foundation.org>
In-Reply-To: <20150513014418.GB14599@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
	<20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
	<20150512084339.GN16501@mwanda>
	<20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
	<20150512091349.GO16501@mwanda>
	<20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
	<20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
	<20150512161511.7967c400cae6c1d693b61d57@linux-foundation.org>
	<20150513014418.GB14599@hori1.linux.bs1.fc.nec.co.jp>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>

On Wed, 13 May 2015 01:44:22 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> > >  			order = huge_page_order(h);
> > >  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> > > +	VM_BUG_ON(order == UINT_MAX);
> > >  	for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << order)
> > >  		dissolve_free_huge_page(pfn_to_page(pfn));
> > 
> > Do we need to calculate this each time?  Can it be done in
> > hugetlb_init_hstates(), save the result in a global?
> 
> Yes, it should work. How about the following?
> This adds 4bytes to .data due to a new global variable, but reduces 47 bytes
> .text size of code reduces, so it's a win in total.
> 
>    text    data     bss     dec     hex filename                         
>   28313     469   84236  113018   1b97a mm/hugetlb.o (above patch)
>   28266     473   84236  112975   1b94f mm/hugetlb.o (below patch)

Looks good.  Please turn it into a real patch and send it over when
convenient?

> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -40,6 +40,7 @@ int hugepages_treat_as_movable;
>  int hugetlb_max_hstate __read_mostly;
>  unsigned int default_hstate_idx;
>  struct hstate hstates[HUGE_MAX_HSTATE];
> +unsigned int minimum_order __read_mostly;

static.

And a comment would be nice ;)

>
> ...
>
> @@ -1626,11 +1621,16 @@ static void __init hugetlb_init_hstates(void)
>  {
>  	struct hstate *h;
>  
> +	minimum_order = UINT_MAX;

Do this at compile time.

>  	for_each_hstate(h) {
> +		if (minimum_order > huge_page_order(h))
> +			minimum_order = huge_page_order(h);
> +
>  		/* oversize hugepages were init'ed in early boot */
>  		if (!hstate_is_gigantic(h))
>  			hugetlb_hstate_alloc_pages(h);
>  	}
> +	VM_BUG_ON(minimum_order == UINT_MAX);

Is the system hopelessly screwed up when this happens, or will it still
be able to boot up and do useful things?

If the system is hopelessly broken then BUG_ON or, better, panic should
be used here.  But if there's still potential to do useful things then
I guess VM_BUG_ON is appropriate.


>  }

--
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>

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <owner-linux-mm@kvack.org>
Received: from mail-ig0-f173.google.com (mail-ig0-f173.google.com [209.85.213.173])
	by kanga.kvack.org (Postfix) with ESMTP id 9BE826B0038
	for <linux-mm@kvack.org>; Thu, 14 May 2015 02:16:25 -0400 (EDT)
Received: by igbhj9 with SMTP id hj9so64963219igb.1
        for <linux-mm@kvack.org>; Wed, 13 May 2015 23:16:25 -0700 (PDT)
Received: from tyo202.gate.nec.co.jp (TYO202.gate.nec.co.jp. [210.143.35.52])
        by mx.google.com with ESMTPS id u27si517220ioi.5.2015.05.13.23.16.24
        for <linux-mm@kvack.org>
        (version=TLSv1 cipher=RC4-SHA bits=128/128);
        Wed, 13 May 2015 23:16:24 -0700 (PDT)
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: [PATCH] mm/hugetlb: introduce minimum hugepage order
Date: Thu, 14 May 2015 06:15:45 +0000
Message-ID: <20150514061543.GA9477@hori1.linux.bs1.fc.nec.co.jp>
References: <20150511111748.GA20660@mwanda>
 <20150511235443.GA8513@hori1.linux.bs1.fc.nec.co.jp>
 <20150512084339.GN16501@mwanda>
 <20150512090454.GD3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512091349.GO16501@mwanda>
 <20150512091640.GE3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512092034.GF3068@hori1.linux.bs1.fc.nec.co.jp>
 <20150512161511.7967c400cae6c1d693b61d57@linux-foundation.org>
 <20150513014418.GB14599@hori1.linux.bs1.fc.nec.co.jp>
 <20150513135556.5d21cd52810f87460eb1f2a1@linux-foundation.org>
In-Reply-To: <20150513135556.5d21cd52810f87460eb1f2a1@linux-foundation.org>
Content-Language: ja-JP
Content-Type: text/plain; charset="iso-2022-jp"
Content-ID: <789A4BFEA78DC54A8E2605348F8D2417@gisp.nec.co.jp>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Sender: owner-linux-mm@kvack.org
List-ID: <linux-mm.kvack.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>, "linux-mm@kvack.org" <linux-mm@kvack.org>

On Wed, May 13, 2015 at 01:55:56PM -0700, Andrew Morton wrote:
> On Wed, 13 May 2015 01:44:22 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec=
.com> wrote:
>=20
> > > >  			order =3D huge_page_order(h);
> > > >  	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
> > > > +	VM_BUG_ON(order =3D=3D UINT_MAX);
> > > >  	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
> > > >  		dissolve_free_huge_page(pfn_to_page(pfn));
> > >=20
> > > Do we need to calculate this each time?  Can it be done in
> > > hugetlb_init_hstates(), save the result in a global?
> >=20
> > Yes, it should work. How about the following?
> > This adds 4bytes to .data due to a new global variable, but reduces 47 =
bytes
> > .text size of code reduces, so it's a win in total.
> >=20
> >    text    data     bss     dec     hex filename                       =
 =20
> >   28313     469   84236  113018   1b97a mm/hugetlb.o (above patch)
> >   28266     473   84236  112975   1b94f mm/hugetlb.o (below patch)
>=20
> Looks good.  Please turn it into a real patch and send it over when
> convenient?

Yes, the patch is attached below.

> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -40,6 +40,7 @@ int hugepages_treat_as_movable;
> >  int hugetlb_max_hstate __read_mostly;
> >  unsigned int default_hstate_idx;
> >  struct hstate hstates[HUGE_MAX_HSTATE];
> > +unsigned int minimum_order __read_mostly;
>=20
> static.
>=20
> And a comment would be nice ;)

OK, fixed.

>=20
> >
> > ...
> >
> > @@ -1626,11 +1621,16 @@ static void __init hugetlb_init_hstates(void)
> >  {
> >  	struct hstate *h;
> > =20
> > +	minimum_order =3D UINT_MAX;
>=20
> Do this at compile time.

OK.

> >  	for_each_hstate(h) {
> > +		if (minimum_order > huge_page_order(h))
> > +			minimum_order =3D huge_page_order(h);
> > +
> >  		/* oversize hugepages were init'ed in early boot */
> >  		if (!hstate_is_gigantic(h))
> >  			hugetlb_hstate_alloc_pages(h);
> >  	}
> > +	VM_BUG_ON(minimum_order =3D=3D UINT_MAX);
>=20
> Is the system hopelessly screwed up when this happens, or will it still
> be able to boot up and do useful things?
>=20
> If the system is hopelessly broken then BUG_ON or, better, panic should
> be used here.  But if there's still potential to do useful things then
> I guess VM_BUG_ON is appropriate.

When this happens, hugetlb subsystem is broken but the system can run as
usual, so this is not critical. So I think VM_BUG_ON is OK.

Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Subject: [PATCH] mm/hugetlb: introduce minimum hugepage order

Currently the initial value of order in dissolve_free_huge_page is 64 or 32=
,
which leads to the following warning in static checker:

  mm/hugetlb.c:1203 dissolve_free_huge_pages()
  warn: potential right shift more than type allows '9,18,64'

This is a potential risk of infinite loop, because 1 << order (=3D=3D 0) is=
 used
in for-loop like this:

  for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
      ...

So this patch fixes it by using global minimum_order calculated at boot tim=
e.

    text    data     bss     dec     hex filename
   28313     469   84236  113018   1b97a mm/hugetlb.o
   28256     473   84236  112965   1b945 mm/hugetlb.o (patched)

Fixes: c8721bbbdd36 ("mm: memory-hotplug: enable memory hotplug to handle h=
ugepage")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/hugetlb.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 271e4432734c..8c4c1f9f9a9a 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -40,6 +40,11 @@ int hugepages_treat_as_movable;
 int hugetlb_max_hstate __read_mostly;
 unsigned int default_hstate_idx;
 struct hstate hstates[HUGE_MAX_HSTATE];
+/*
+ * Minimum page order among possible hugepage sizes, set to a proper value
+ * at boot time.
+ */
+static unsigned int minimum_order __read_mostly =3D UINT_MAX;
=20
 __initdata LIST_HEAD(huge_boot_pages);
=20
@@ -1188,19 +1193,13 @@ static void dissolve_free_huge_page(struct page *pa=
ge)
  */
 void dissolve_free_huge_pages(unsigned long start_pfn, unsigned long end_p=
fn)
 {
-	unsigned int order =3D 8 * sizeof(void *);
 	unsigned long pfn;
-	struct hstate *h;
=20
 	if (!hugepages_supported())
 		return;
=20
-	/* Set scan step to minimum hugepage size */
-	for_each_hstate(h)
-		if (order > huge_page_order(h))
-			order =3D huge_page_order(h);
-	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << order));
-	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << order)
+	VM_BUG_ON(!IS_ALIGNED(start_pfn, 1 << minimum_order));
+	for (pfn =3D start_pfn; pfn < end_pfn; pfn +=3D 1 << minimum_order)
 		dissolve_free_huge_page(pfn_to_page(pfn));
 }
=20
@@ -1627,10 +1626,14 @@ static void __init hugetlb_init_hstates(void)
 	struct hstate *h;
=20
 	for_each_hstate(h) {
+		if (minimum_order > huge_page_order(h))
+			minimum_order =3D huge_page_order(h);
+
 		/* oversize hugepages were init'ed in early boot */
 		if (!hstate_is_gigantic(h))
 			hugetlb_hstate_alloc_pages(h);
 	}
+	VM_BUG_ON(minimum_order =3D=3D UINT_MAX);
 }
=20
 static char * __init memfmt(char *buf, unsigned long n)
--=20
2.1.0

--
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>