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.
Signed-off-by: Petr Holasek <[email protected]>
---
mm/hugetlb.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index bb0b7c1..f99d7a8 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -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;
--
1.7.1
On Mon, Feb 21, 2011 at 04:47:49PM +0100, Petr Holasek 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 (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.
>
> Signed-off-by: Petr Holasek <[email protected]>
> ---
> mm/hugetlb.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index bb0b7c1..f99d7a8 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -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;
Looks reasonable.
hugetlb_overcommit_handler() has the same wrong behavior.
So how about fixing that too?
Anyway,
Reviewed-by: Naoya Horiguchi <[email protected]>
On Mon, Feb 21, 2011 at 04:47:49PM +0100, Petr Holasek 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 (can be easily showed
> at /proc/meminfo output).
I bet you a shiny penny that the value of HugePages becomes the maximum
number that could be allocated by the system at the time rather than a
random value.
> This patch fixes the wrong behavior so that the
> negative input will result in nr_hugepages value unchanged.
>
As pointed out elsewhere, nr_overcommit_hugepages also needs fixing but
the fix is essentially the same.
> Signed-off-by: Petr Holasek <[email protected]>
> ---
> mm/hugetlb.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index bb0b7c1..f99d7a8 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -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;
> --
> 1.7.1
>
--
Mel Gorman
SUSE Labs
On Tue, 22 Feb 2011 10:02:36 +0000
Mel Gorman <[email protected]> wrote:
> On Mon, Feb 21, 2011 at 04:47:49PM +0100, Petr Holasek 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 (can be easily showed
> > at /proc/meminfo output).
>
> I bet you a shiny penny that the value of HugePages becomes the maximum
> number that could be allocated by the system at the time rather than a
> random value.
That seems to be the case from my reading. In which case the patch
removes probably-undocumented and possibly-useful existing behavior.
On Wed, Feb 23, 2011 at 04:18:18PM -0800, Andrew Morton wrote:
> On Tue, 22 Feb 2011 10:02:36 +0000
> Mel Gorman <[email protected]> wrote:
>
> > On Mon, Feb 21, 2011 at 04:47:49PM +0100, Petr Holasek 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 (can be easily showed
> > > at /proc/meminfo output).
> >
> > I bet you a shiny penny that the value of HugePages becomes the maximum
> > number that could be allocated by the system at the time rather than a
> > random value.
>
> That seems to be the case from my reading. In which case the patch
> removes probably-undocumented and possibly-useful existing behavior.
>
It's not proof that no one does this but I'm not aware of any documentation
related to hugetlbfs that recommends writing negative values to take advantage
of this side-effect. It's more likely they simply wrote a very large number
to nr_hugepages if they wanted "as many hugepages as possible" as it makes
more intuitive sense than asking for a negative amount of pages. hugeadm at
least is not depending on this behaviour AFAIK.
--
Mel Gorman
SUSE Labs
On Thu, 24 Feb 2011, Mel Gorman wrote:
> On Wed, Feb 23, 2011 at 04:18:18PM -0800, Andrew Morton wrote:
> > On Tue, 22 Feb 2011 10:02:36 +0000
> > Mel Gorman <[email protected]> wrote:
> >
> > > On Mon, Feb 21, 2011 at 04:47:49PM +0100, Petr Holasek 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 (can be easily showed
> > > > at /proc/meminfo output).
> > >
> > > I bet you a shiny penny that the value of HugePages becomes the maximum
> > > number that could be allocated by the system at the time rather than a
> > > random value.
> >
> > That seems to be the case from my reading. In which case the patch
> > removes probably-undocumented and possibly-useful existing behavior.
> >
>
> It's not proof that no one does this but I'm not aware of any documentation
> related to hugetlbfs that recommends writing negative values to take advantage
> of this side-effect. It's more likely they simply wrote a very large number
> to nr_hugepages if they wanted "as many hugepages as possible" as it makes
> more intuitive sense than asking for a negative amount of pages. hugeadm at
> least is not depending on this behaviour AFAIK.
That is correct, hugeadm never writes negative values to huge page pool sizes.