hugetlbfs: correct handling of negative input to /proc/sys/vm/nr_hugepages

PATCH In review Petr Holasek 6 participants 9 messages Feb 22, 2011 – Feb 25, 2011

Subsystems (2)

Memory management HugeTLB

lore

Share & tools

lore

Petr Holasek

author

2011-02-22 20:17 UTC #0 lore raw

PATCH v2 1/1 hugetlbfs: correct handling of negative input to /proc/sys/vm/nr_hugepages

When user insert negative value into /proc/sys/vm/nr_hugepages it will result in the setting a random number of HugePages in system (can be easily showed at /proc/meminfo output). This patch fixes the wrong behavior so that the negative input will result in nr_hugepages value unchanged.

v2: same fix was also done in hugetlb_overcommit_handler function
as suggested by reviewers.

Signed-off-by Petr Holasek <[email protected]>

Reviewed-by Naoya Horiguchi <[email protected]>

---
 mm/hugetlb.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

mm/hugetlb.c +2 -4

index bb0b7c1..06de5aa 100644
@@ -1872,8 +1872,7 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
     unsigned long tmp;
     int ret;
 
-    if (!write)
-        tmp = h->max_huge_pages;
+    tmp = h->max_huge_pages;
 
     if (write && h->order >= MAX_ORDER)
         return -EINVAL;

@@ -1938,8 +1937,7 @@ int hugetlb_overcommit_handler(struct ctl_table *table, int write,
     unsigned long tmp;
     int ret;
 
-    if (!write)
-        tmp = h->nr_overcommit_huge_pages;
+    tmp = h->nr_overcommit_huge_pages;
 
     if (write && h->order >= MAX_ORDER)
         return -EINVAL;

Signature · 8 lines

1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

Acked-by on v2 1/1

2011-02-22 20:32 UTC #1 lore raw

Quoted 10 lines · Petr Holasek

When user insert negative value into /proc/sys/vm/nr_hugepages it will result in the setting a random number of HugePages in system (can be easily showed at /proc/meminfo output). This patch fixes the wrong behavior so that the negative input will result in nr_hugepages value unchanged.
v2: same fix was also done in hugetlb_overcommit_handler function
as suggested by reviewers.
Signed-off-by Petr Holasek <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

Acked-by David Rientjes <[email protected]>

Mel Gorman

Acked-by on v2 1/1

2011-02-23 09:01 UTC #2 lore raw

In reply to Petr Holasek

Quoted 11 lines · Petr Holasek

When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system (can be easily showed at /proc/meminfo output). This patch fixes the wrong behavior so that the negative input will result in nr_hugepages value unchanged.
v2: same fix was also done in hugetlb_overcommit_handler function
as suggested by reviewers.
Signed-off-by Petr Holasek <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

Thanks

Acked-by Mel Gorman <[email protected]>

Signature · 2 lines

Mel Gorman
SUSE Labs

Eric B Munson

Acked-by on v2 1/1

2011-02-23 15:34 UTC #3 lore raw

In reply to Petr Holasek

Quoted 11 lines · Petr Holasek

When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system (can be easily showed at /proc/meminfo output). This patch fixes the wrong behavior so that the negative input will result in nr_hugepages value unchanged.
v2: same fix was also done in hugetlb_overcommit_handler function
as suggested by reviewers.
Signed-off-by Petr Holasek <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>

Acked-by Eric B Munson <[email protected]>

Anton Arapov

2011-02-24 08:16 UTC #4 lore raw

In reply to Petr Holasek

Quoted 40 lines · Petr Holasek

When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system (can be easily showed at /proc/meminfo output). This patch fixes the wrong behavior so that the negative input will result in nr_hugepages value unchanged.
v2: same fix was also done in hugetlb_overcommit_handler function
as suggested by reviewers.
Signed-off-by Petr Holasek <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>
---
 mm/hugetlb.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
mm/hugetlb.c +1 -2
index bb0b7c1..06de5aa 100644
@@ -1872,8 +1872,7 @@ static int hugetlb_sysctl_handler_common(bool
 obey_mempolicy,
     unsigned long tmp;
     int ret;
 
-    if (!write)
-        tmp = h->max_huge_pages;
+    tmp = h->max_huge_pages;
 
     if (write && h->order >= MAX_ORDER)
         return -EINVAL;
mm/hugetlb.c +1 -2
@@ -1938,8 +1937,7 @@ int hugetlb_overcommit_handler(struct
 ctl_table *table, int write,
     unsigned long tmp;
     int ret;
 
-    if (!write)
-        tmp = h->nr_overcommit_huge_pages;
+    tmp = h->nr_overcommit_huge_pages;
 
     if (write && h->order >= MAX_ORDER)

Quoted 9 lines

         return -EINVAL;

Signature · 6 lines

1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Signed-off-by Anton Arapov <[email protected]>

Signature · 8 lines

Anton

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Petr Holasek

author

2011-02-24 17:02 UTC #5 lore raw

In reply to Petr Holasek

----- Original Message -----

Quoted 50 lines

From: "Petr Holasek" <[email protected]>
To: [email protected]
Cc: "Petr Holasek" <[email protected]>, "Andi Kleen" <[email protected]>, "Naoya Horiguchi" <[email protected]>, "Mel Gorman" <[email protected]>, "Andrew Morton" <[email protected]>, "Wu Fengguang" <[email protected]>, [email protected]
Sent: Tuesday, February 22, 2011 9:17:04 PM
Subject: [PATCH v2] hugetlbfs: correct handling of negative input to /proc/sys/vm/nr_hugepages
When user insert negative value into /proc/sys/vm/nr_hugepages it will
result
in the setting a random number of HugePages in system (can be easily
showed
at /proc/meminfo output). This patch fixes the wrong behavior so that
the
negative input will result in nr_hugepages value unchanged.
v2: same fix was also done in hugetlb_overcommit_handler function as suggested by reviewers.
Signed-off-by Petr Holasek <[email protected]>
Reviewed-by Naoya Horiguchi <[email protected]>
---
mm/hugetlb.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
mm/hugetlb.c +1 -2
index bb0b7c1..06de5aa 100644
@@ -1872,8 +1872,7 @@ static int hugetlb_sysctl_handler_common(bool
 obey_mempolicy,
 unsigned long tmp;
 int ret;
 
- if (!write)
- tmp = h->max_huge_pages;
+ tmp = h->max_huge_pages;
 
 if (write && h->order >= MAX_ORDER)
return -EINVAL;
mm/hugetlb.c +1 -2
@@ -1938,8 +1937,7 @@ int hugetlb_overcommit_handler(struct ctl_table
 *table, int write,
 unsigned long tmp;
 int ret;
 
- if (!write)
- tmp = h->nr_overcommit_huge_pages;
+ tmp = h->nr_overcommit_huge_pages;
 
 if (write && h->order >= MAX_ORDER)
return -EINVAL;

Quoted 8 lines

--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

I am really sorry, but tabs were broken up into spaces during posting, here is the right version:

thanks,
Petr

---
 mm/hugetlb.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

mm/hugetlb.c +2 -4

index bb0b7c1..06de5aa 100644
@@ -1872,8 +1872,7 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
 	unsigned long tmp;
 	int ret;
 
-	if (!write)
-		tmp = h->max_huge_pages;
+	tmp = h->max_huge_pages;
 
 	if (write && h->order >= MAX_ORDER)
 		return -EINVAL;

@@ -1938,8 +1937,7 @@ int hugetlb_overcommit_handler(struct ctl_table *table, int write,
 	unsigned long tmp;
 	int ret;
 
-	if (!write)
-		tmp = h->nr_overcommit_huge_pages;
+	tmp = h->nr_overcommit_huge_pages;
 
 	if (write && h->order >= MAX_ORDER)
 		return -EINVAL;

Signature · 8 lines

1.7.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrew Morton

maintainer

2011-02-24 22:11 UTC #6 lore raw

In reply to Petr Holasek

Quoted 3 lines · On Tue, 22 Feb 2011 21:17:04 +0100 Petr Holasek <[email protected]> wrote:

When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system

Is this true? afacit the kernel will allocate as many pages as it can and will then set /proc/sys/vm/nr_hugepages to reflect the result. That's not random.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Andrew Morton

maintainer

2011-02-24 22:14 UTC #7 lore raw

Quoted 11 lines · On Thu, 24 Feb 2011 14:10:34 -0800 Andrew Morton <[email protected]>…

Quoted 3 lines · On Tue, 22 Feb 2011 21:17:04 +0100 Petr Holasek <[email protected]> wrote:
When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system
Is this true? afacit the kernel will allocate as many pages as it can and will then set /proc/sys/vm/nr_hugepages to reflect the result. That's not random.

Assuming the above to be correct, I altered the changelog thusly:

: When the user inserts a negative value into /proc/sys/vm/nr_hugepages it : will cause the kernel to allocate as many hugepages as possible and to : then update /proc/meminfo to reflect this.
:
: This changes the behavior so that the negative input will result in : nr_hugepages value being unchanged.

and given that, I don't really see why we should change the existing behaviour.

Mel Gorman

2011-02-25 17:30 UTC #8 lore raw

Quoted 10 lines · Andrew Morton

Quoted 7 lines · On Thu, 24 Feb 2011 14:10:34 -0800 Andrew Morton <[email protected]>…
Quoted 3 lines · On Tue, 22 Feb 2011 21:17:04 +0100 Petr Holasek <[email protected]> wrote:
When user insert negative value into /proc/sys/vm/nr_hugepages it will result
in the setting a random number of HugePages in system

Quoted 7 lines

Quoted 4 lines
Is this true? afacit the kernel will allocate as many pages as it can and will then set /proc/sys/vm/nr_hugepages to reflect the result. That's not random.
Assuming the above to be correct, I altered the changelog thusly:

AFAIK, it's correct.

Quoted 9 lines

: When the user inserts a negative value into /proc/sys/vm/nr_hugepages it : will cause the kernel to allocate as many hugepages as possible and to : then update /proc/meminfo to reflect this.
:
: This changes the behavior so that the negative input will result in : nr_hugepages value being unchanged.
and given that, I don't really see why we should change the existing behaviour.

The main motivation is that asking the kernel for -1 pages and getting a sensible response just feels wrong. The second reason I'd guess is that an administrator script that was buggy (or raced with a second) instance that accidentally wrote a negative number to the proc interface would try allocating all memory as huge pages instead of reducing the number of hugepages as was probably intended. Totally hypothetical case of course, I haven't actually heard of this happening to anyone.

Signature · 2 lines

Mel Gorman
SUSE Labs