Three different interfaces alter the maximum number of hugepages for an
hstate:
- /proc/sys/vm/nr_hugepages for global number of hugepages of the default
hstate,
- /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages for global number of
hugepages for a specific hstate, and
- /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages/mempolicy for number of
hugepages for a specific hstate over the set of allowed nodes.
Generalize the code so that a single function handles all of these writes
instead of duplicating the code in two different functions.
This decreases the number of lines of code, but also reduces the size of
.text by about half a percent since set_max_huge_pages() can be inlined.
Signed-off-by: David Rientjes <[email protected]>
---
mm/hugetlb.c | 61 ++++++++++++++++++++++++++----------------------------------
1 file changed, 26 insertions(+), 35 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1734,21 +1734,13 @@ static ssize_t nr_hugepages_show_common(struct kobject *kobj,
return sprintf(buf, "%lu\n", nr_huge_pages);
}
-static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
- struct kobject *kobj, struct kobj_attribute *attr,
- const char *buf, size_t len)
+static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
+ struct hstate *h, int nid,
+ unsigned long count, size_t len)
{
int err;
- int nid;
- unsigned long count;
- struct hstate *h;
NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
- err = kstrtoul(buf, 10, &count);
- if (err)
- goto out;
-
- h = kobj_to_hstate(kobj, &nid);
if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
err = -EINVAL;
goto out;
@@ -1784,6 +1776,23 @@ out:
return err;
}
+static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
+ struct kobject *kobj, const char *buf,
+ size_t len)
+{
+ struct hstate *h;
+ unsigned long count;
+ int nid;
+ int err;
+
+ err = kstrtoul(buf, 10, &count);
+ if (err)
+ return err;
+
+ h = kobj_to_hstate(kobj, &nid);
+ return __nr_hugepages_store_common(obey_mempolicy, h, nid, count, len);
+}
+
static ssize_t nr_hugepages_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
{
@@ -1793,7 +1802,7 @@ static ssize_t nr_hugepages_show(struct kobject *kobj,
static ssize_t nr_hugepages_store(struct kobject *kobj,
struct kobj_attribute *attr, const char *buf, size_t len)
{
- return nr_hugepages_store_common(false, kobj, attr, buf, len);
+ return nr_hugepages_store_common(false, kobj, buf, len);
}
HSTATE_ATTR(nr_hugepages);
@@ -1812,7 +1821,7 @@ static ssize_t nr_hugepages_mempolicy_show(struct kobject *kobj,
static ssize_t nr_hugepages_mempolicy_store(struct kobject *kobj,
struct kobj_attribute *attr, const char *buf, size_t len)
{
- return nr_hugepages_store_common(true, kobj, attr, buf, len);
+ return nr_hugepages_store_common(true, kobj, buf, len);
}
HSTATE_ATTR(nr_hugepages_mempolicy);
#endif
@@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
void __user *buffer, size_t *length, loff_t *ppos)
{
struct hstate *h = &default_hstate;
- unsigned long tmp;
+ unsigned long tmp = h->max_huge_pages;
int ret;
- if (!hugepages_supported())
- return -ENOTSUPP;
-
- tmp = h->max_huge_pages;
-
- if (write && hstate_is_gigantic(h) && !gigantic_page_supported())
- return -EINVAL;
-
table->data = &tmp;
table->maxlen = sizeof(unsigned long);
ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
if (ret)
goto out;
- if (write) {
- NODEMASK_ALLOC(nodemask_t, nodes_allowed,
- GFP_KERNEL | __GFP_NORETRY);
- if (!(obey_mempolicy &&
- init_nodemask_of_mempolicy(nodes_allowed))) {
- NODEMASK_FREE(nodes_allowed);
- nodes_allowed = &node_states[N_MEMORY];
- }
- h->max_huge_pages = set_max_huge_pages(h, tmp, nodes_allowed);
-
- if (nodes_allowed != &node_states[N_MEMORY])
- NODEMASK_FREE(nodes_allowed);
- }
+ if (write)
+ ret = __nr_hugepages_store_common(obey_mempolicy, h,
+ NUMA_NO_NODE, tmp, *length);
out:
return ret;
}
They are unnecessary: "zero" can be used in place of "hugetlb_zero" and
passing extra2 == NULL is equivalent to infinity.
Signed-off-by: David Rientjes <[email protected]>
---
include/linux/hugetlb.h | 1 -
kernel/sysctl.c | 9 +++------
mm/hugetlb.c | 1 -
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -86,7 +86,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud);
#endif
extern unsigned long hugepages_treat_as_movable;
-extern const unsigned long hugetlb_zero, hugetlb_infinity;
extern int sysctl_hugetlb_shm_group;
extern struct list_head huge_boot_pages;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1240,8 +1240,7 @@ static struct ctl_table vm_table[] = {
.maxlen = sizeof(unsigned long),
.mode = 0644,
.proc_handler = hugetlb_sysctl_handler,
- .extra1 = (void *)&hugetlb_zero,
- .extra2 = (void *)&hugetlb_infinity,
+ .extra1 = &zero,
},
#ifdef CONFIG_NUMA
{
@@ -1250,8 +1249,7 @@ static struct ctl_table vm_table[] = {
.maxlen = sizeof(unsigned long),
.mode = 0644,
.proc_handler = &hugetlb_mempolicy_sysctl_handler,
- .extra1 = (void *)&hugetlb_zero,
- .extra2 = (void *)&hugetlb_infinity,
+ .extra1 = &zero,
},
#endif
{
@@ -1274,8 +1272,7 @@ static struct ctl_table vm_table[] = {
.maxlen = sizeof(unsigned long),
.mode = 0644,
.proc_handler = hugetlb_overcommit_handler,
- .extra1 = (void *)&hugetlb_zero,
- .extra2 = (void *)&hugetlb_infinity,
+ .extra1 = &zero,
},
#endif
{
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -35,7 +35,6 @@
#include <linux/node.h>
#include "internal.h"
-const unsigned long hugetlb_zero = 0, hugetlb_infinity = ~0UL;
unsigned long hugepages_treat_as_movable;
int hugetlb_max_hstate __read_mostly;
On Mon, Jun 30, 2014 at 04:57:06PM -0700, David Rientjes wrote:
> Three different interfaces alter the maximum number of hugepages for an
> hstate:
>
> - /proc/sys/vm/nr_hugepages for global number of hugepages of the default
> hstate,
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages for global number of
> hugepages for a specific hstate, and
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages/mempolicy for number of
> hugepages for a specific hstate over the set of allowed nodes.
>
> Generalize the code so that a single function handles all of these writes
> instead of duplicating the code in two different functions.
>
> This decreases the number of lines of code, but also reduces the size of
> .text by about half a percent since set_max_huge_pages() can be inlined.
>
> Signed-off-by: David Rientjes <[email protected]>
Looks good to me.
Reviewed-by: Naoya Horiguchi <[email protected]>
> ---
> mm/hugetlb.c | 61 ++++++++++++++++++++++++++----------------------------------
> 1 file changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1734,21 +1734,13 @@ static ssize_t nr_hugepages_show_common(struct kobject *kobj,
> return sprintf(buf, "%lu\n", nr_huge_pages);
> }
>
> -static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> - struct kobject *kobj, struct kobj_attribute *attr,
> - const char *buf, size_t len)
> +static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
> + struct hstate *h, int nid,
> + unsigned long count, size_t len)
> {
> int err;
> - int nid;
> - unsigned long count;
> - struct hstate *h;
> NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
>
> - err = kstrtoul(buf, 10, &count);
> - if (err)
> - goto out;
> -
> - h = kobj_to_hstate(kobj, &nid);
> if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> err = -EINVAL;
> goto out;
> @@ -1784,6 +1776,23 @@ out:
> return err;
> }
>
> +static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> + struct kobject *kobj, const char *buf,
> + size_t len)
> +{
> + struct hstate *h;
> + unsigned long count;
> + int nid;
> + int err;
> +
> + err = kstrtoul(buf, 10, &count);
> + if (err)
> + return err;
> +
> + h = kobj_to_hstate(kobj, &nid);
> + return __nr_hugepages_store_common(obey_mempolicy, h, nid, count, len);
> +}
> +
> static ssize_t nr_hugepages_show(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> @@ -1793,7 +1802,7 @@ static ssize_t nr_hugepages_show(struct kobject *kobj,
> static ssize_t nr_hugepages_store(struct kobject *kobj,
> struct kobj_attribute *attr, const char *buf, size_t len)
> {
> - return nr_hugepages_store_common(false, kobj, attr, buf, len);
> + return nr_hugepages_store_common(false, kobj, buf, len);
> }
> HSTATE_ATTR(nr_hugepages);
>
> @@ -1812,7 +1821,7 @@ static ssize_t nr_hugepages_mempolicy_show(struct kobject *kobj,
> static ssize_t nr_hugepages_mempolicy_store(struct kobject *kobj,
> struct kobj_attribute *attr, const char *buf, size_t len)
> {
> - return nr_hugepages_store_common(true, kobj, attr, buf, len);
> + return nr_hugepages_store_common(true, kobj, buf, len);
> }
> HSTATE_ATTR(nr_hugepages_mempolicy);
> #endif
> @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> void __user *buffer, size_t *length, loff_t *ppos)
> {
> struct hstate *h = &default_hstate;
> - unsigned long tmp;
> + unsigned long tmp = h->max_huge_pages;
> int ret;
>
> - if (!hugepages_supported())
> - return -ENOTSUPP;
> -
> - tmp = h->max_huge_pages;
> -
> - if (write && hstate_is_gigantic(h) && !gigantic_page_supported())
> - return -EINVAL;
> -
> table->data = &tmp;
> table->maxlen = sizeof(unsigned long);
> ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> if (ret)
> goto out;
>
> - if (write) {
> - NODEMASK_ALLOC(nodemask_t, nodes_allowed,
> - GFP_KERNEL | __GFP_NORETRY);
> - if (!(obey_mempolicy &&
> - init_nodemask_of_mempolicy(nodes_allowed))) {
> - NODEMASK_FREE(nodes_allowed);
> - nodes_allowed = &node_states[N_MEMORY];
> - }
> - h->max_huge_pages = set_max_huge_pages(h, tmp, nodes_allowed);
> -
> - if (nodes_allowed != &node_states[N_MEMORY])
> - NODEMASK_FREE(nodes_allowed);
> - }
> + if (write)
> + ret = __nr_hugepages_store_common(obey_mempolicy, h,
> + NUMA_NO_NODE, tmp, *length);
> out:
> return ret;
> }
>
> --
> 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/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>
On Mon, Jun 30, 2014 at 05:46:35PM -0700, David Rientjes wrote:
> They are unnecessary: "zero" can be used in place of "hugetlb_zero" and
> passing extra2 == NULL is equivalent to infinity.
>
> Signed-off-by: David Rientjes <[email protected]>
Reviewed-by: Naoya Horiguchi <[email protected]>
> ---
> include/linux/hugetlb.h | 1 -
> kernel/sysctl.c | 9 +++------
> mm/hugetlb.c | 1 -
> 3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -86,7 +86,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud);
> #endif
>
> extern unsigned long hugepages_treat_as_movable;
> -extern const unsigned long hugetlb_zero, hugetlb_infinity;
> extern int sysctl_hugetlb_shm_group;
> extern struct list_head huge_boot_pages;
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1240,8 +1240,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = hugetlb_sysctl_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #ifdef CONFIG_NUMA
> {
> @@ -1250,8 +1249,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = &hugetlb_mempolicy_sysctl_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #endif
> {
> @@ -1274,8 +1272,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = hugetlb_overcommit_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #endif
> {
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -35,7 +35,6 @@
> #include <linux/node.h>
> #include "internal.h"
>
> -const unsigned long hugetlb_zero = 0, hugetlb_infinity = ~0UL;
> unsigned long hugepages_treat_as_movable;
>
> int hugetlb_max_hstate __read_mostly;
>
> --
> 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/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>
On Mon, 30 Jun 2014 16:57:06 -0700 (PDT)
David Rientjes <[email protected]> wrote:
> Three different interfaces alter the maximum number of hugepages for an
> hstate:
>
> - /proc/sys/vm/nr_hugepages for global number of hugepages of the default
> hstate,
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages for global number of
> hugepages for a specific hstate, and
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages/mempolicy for number of
> hugepages for a specific hstate over the set of allowed nodes.
>
> Generalize the code so that a single function handles all of these writes
> instead of duplicating the code in two different functions.
>
> This decreases the number of lines of code, but also reduces the size of
> .text by about half a percent since set_max_huge_pages() can be inlined.
>
> Signed-off-by: David Rientjes <[email protected]>
> ---
> mm/hugetlb.c | 61 ++++++++++++++++++++++++++----------------------------------
> 1 file changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1734,21 +1734,13 @@ static ssize_t nr_hugepages_show_common(struct kobject *kobj,
> return sprintf(buf, "%lu\n", nr_huge_pages);
> }
>
> -static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> - struct kobject *kobj, struct kobj_attribute *attr,
> - const char *buf, size_t len)
> +static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
> + struct hstate *h, int nid,
> + unsigned long count, size_t len)
> {
> int err;
> - int nid;
> - unsigned long count;
> - struct hstate *h;
> NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
>
> - err = kstrtoul(buf, 10, &count);
> - if (err)
> - goto out;
> -
> - h = kobj_to_hstate(kobj, &nid);
> if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> err = -EINVAL;
> goto out;
> @@ -1784,6 +1776,23 @@ out:
> return err;
> }
>
> +static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> + struct kobject *kobj, const char *buf,
> + size_t len)
> +{
> + struct hstate *h;
> + unsigned long count;
> + int nid;
> + int err;
> +
> + err = kstrtoul(buf, 10, &count);
> + if (err)
> + return err;
> +
> + h = kobj_to_hstate(kobj, &nid);
> + return __nr_hugepages_store_common(obey_mempolicy, h, nid, count, len);
> +}
> +
> static ssize_t nr_hugepages_show(struct kobject *kobj,
> struct kobj_attribute *attr, char *buf)
> {
> @@ -1793,7 +1802,7 @@ static ssize_t nr_hugepages_show(struct kobject *kobj,
> static ssize_t nr_hugepages_store(struct kobject *kobj,
> struct kobj_attribute *attr, const char *buf, size_t len)
> {
> - return nr_hugepages_store_common(false, kobj, attr, buf, len);
> + return nr_hugepages_store_common(false, kobj, buf, len);
> }
> HSTATE_ATTR(nr_hugepages);
>
> @@ -1812,7 +1821,7 @@ static ssize_t nr_hugepages_mempolicy_show(struct kobject *kobj,
> static ssize_t nr_hugepages_mempolicy_store(struct kobject *kobj,
> struct kobj_attribute *attr, const char *buf, size_t len)
> {
> - return nr_hugepages_store_common(true, kobj, attr, buf, len);
> + return nr_hugepages_store_common(true, kobj, buf, len);
> }
> HSTATE_ATTR(nr_hugepages_mempolicy);
> #endif
> @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> void __user *buffer, size_t *length, loff_t *ppos)
> {
> struct hstate *h = &default_hstate;
> - unsigned long tmp;
> + unsigned long tmp = h->max_huge_pages;
> int ret;
>
> - if (!hugepages_supported())
> - return -ENOTSUPP;
Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
looks good to me.
> -
> - tmp = h->max_huge_pages;
> -
> - if (write && hstate_is_gigantic(h) && !gigantic_page_supported())
> - return -EINVAL;
> -
> table->data = &tmp;
> table->maxlen = sizeof(unsigned long);
> ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> if (ret)
> goto out;
>
> - if (write) {
> - NODEMASK_ALLOC(nodemask_t, nodes_allowed,
> - GFP_KERNEL | __GFP_NORETRY);
> - if (!(obey_mempolicy &&
> - init_nodemask_of_mempolicy(nodes_allowed))) {
> - NODEMASK_FREE(nodes_allowed);
> - nodes_allowed = &node_states[N_MEMORY];
> - }
> - h->max_huge_pages = set_max_huge_pages(h, tmp, nodes_allowed);
> -
> - if (nodes_allowed != &node_states[N_MEMORY])
> - NODEMASK_FREE(nodes_allowed);
> - }
> + if (write)
> + ret = __nr_hugepages_store_common(obey_mempolicy, h,
> + NUMA_NO_NODE, tmp, *length);
> out:
> return ret;
> }
>
On Mon, 30 Jun 2014 17:46:35 -0700 (PDT)
David Rientjes <[email protected]> wrote:
> They are unnecessary: "zero" can be used in place of "hugetlb_zero" and
> passing extra2 == NULL is equivalent to infinity.
>
> Signed-off-by: David Rientjes <[email protected]>
Reviewed-by: Luiz Capitulino <[email protected]>
> ---
> include/linux/hugetlb.h | 1 -
> kernel/sysctl.c | 9 +++------
> mm/hugetlb.c | 1 -
> 3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -86,7 +86,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud);
> #endif
>
> extern unsigned long hugepages_treat_as_movable;
> -extern const unsigned long hugetlb_zero, hugetlb_infinity;
> extern int sysctl_hugetlb_shm_group;
> extern struct list_head huge_boot_pages;
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1240,8 +1240,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = hugetlb_sysctl_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #ifdef CONFIG_NUMA
> {
> @@ -1250,8 +1249,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = &hugetlb_mempolicy_sysctl_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #endif
> {
> @@ -1274,8 +1272,7 @@ static struct ctl_table vm_table[] = {
> .maxlen = sizeof(unsigned long),
> .mode = 0644,
> .proc_handler = hugetlb_overcommit_handler,
> - .extra1 = (void *)&hugetlb_zero,
> - .extra2 = (void *)&hugetlb_infinity,
> + .extra1 = &zero,
> },
> #endif
> {
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -35,7 +35,6 @@
> #include <linux/node.h>
> #include "internal.h"
>
> -const unsigned long hugetlb_zero = 0, hugetlb_infinity = ~0UL;
> unsigned long hugepages_treat_as_movable;
>
> int hugetlb_max_hstate __read_mostly;
>
On Wed, 2 Jul 2014, Luiz Capitulino wrote:
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1734,21 +1734,13 @@ static ssize_t nr_hugepages_show_common(struct kobject *kobj,
> > return sprintf(buf, "%lu\n", nr_huge_pages);
> > }
> >
> > -static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> > - struct kobject *kobj, struct kobj_attribute *attr,
> > - const char *buf, size_t len)
> > +static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
> > + struct hstate *h, int nid,
> > + unsigned long count, size_t len)
> > {
> > int err;
> > - int nid;
> > - unsigned long count;
> > - struct hstate *h;
> > NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
> >
> > - err = kstrtoul(buf, 10, &count);
> > - if (err)
> > - goto out;
> > -
> > - h = kobj_to_hstate(kobj, &nid);
> > if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> > err = -EINVAL;
> > goto out;
> > @@ -1784,6 +1776,23 @@ out:
> > return err;
> > }
> >
> > +static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> > + struct kobject *kobj, const char *buf,
> > + size_t len)
> > +{
> > + struct hstate *h;
> > + unsigned long count;
> > + int nid;
> > + int err;
> > +
> > + err = kstrtoul(buf, 10, &count);
> > + if (err)
> > + return err;
> > +
> > + h = kobj_to_hstate(kobj, &nid);
> > + return __nr_hugepages_store_common(obey_mempolicy, h, nid, count, len);
> > +}
> > +
> > static ssize_t nr_hugepages_show(struct kobject *kobj,
> > struct kobj_attribute *attr, char *buf)
> > {
> > @@ -1793,7 +1802,7 @@ static ssize_t nr_hugepages_show(struct kobject *kobj,
> > static ssize_t nr_hugepages_store(struct kobject *kobj,
> > struct kobj_attribute *attr, const char *buf, size_t len)
> > {
> > - return nr_hugepages_store_common(false, kobj, attr, buf, len);
> > + return nr_hugepages_store_common(false, kobj, buf, len);
> > }
> > HSTATE_ATTR(nr_hugepages);
> >
> > @@ -1812,7 +1821,7 @@ static ssize_t nr_hugepages_mempolicy_show(struct kobject *kobj,
> > static ssize_t nr_hugepages_mempolicy_store(struct kobject *kobj,
> > struct kobj_attribute *attr, const char *buf, size_t len)
> > {
> > - return nr_hugepages_store_common(true, kobj, attr, buf, len);
> > + return nr_hugepages_store_common(true, kobj, buf, len);
> > }
> > HSTATE_ATTR(nr_hugepages_mempolicy);
> > #endif
> > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > void __user *buffer, size_t *length, loff_t *ppos)
> > {
> > struct hstate *h = &default_hstate;
> > - unsigned long tmp;
> > + unsigned long tmp = h->max_huge_pages;
> > int ret;
> >
> > - if (!hugepages_supported())
> > - return -ENOTSUPP;
>
> Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> looks good to me.
>
Hmm, I think you're right but I don't think __nr_hugepages_store_common()
is the right place: if we have a legitimate hstate for the sysfs tunables
then we should support hugepages. I think this should be kept in
hugetlb_sysctl_handler_common().
> > -
> > - tmp = h->max_huge_pages;
> > -
> > - if (write && hstate_is_gigantic(h) && !gigantic_page_supported())
> > - return -EINVAL;
> > -
> > table->data = &tmp;
> > table->maxlen = sizeof(unsigned long);
> > ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> > if (ret)
> > goto out;
> >
> > - if (write) {
> > - NODEMASK_ALLOC(nodemask_t, nodes_allowed,
> > - GFP_KERNEL | __GFP_NORETRY);
> > - if (!(obey_mempolicy &&
> > - init_nodemask_of_mempolicy(nodes_allowed))) {
> > - NODEMASK_FREE(nodes_allowed);
> > - nodes_allowed = &node_states[N_MEMORY];
> > - }
> > - h->max_huge_pages = set_max_huge_pages(h, tmp, nodes_allowed);
> > -
> > - if (nodes_allowed != &node_states[N_MEMORY])
> > - NODEMASK_FREE(nodes_allowed);
> > - }
> > + if (write)
> > + ret = __nr_hugepages_store_common(obey_mempolicy, h,
> > + NUMA_NO_NODE, tmp, *length);
> > out:
> > return ret;
> > }
On Mon, 2014-06-30 at 16:57 -0700, David Rientjes wrote:
> Three different interfaces alter the maximum number of hugepages for an
> hstate:
>
> - /proc/sys/vm/nr_hugepages for global number of hugepages of the default
> hstate,
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages for global number of
> hugepages for a specific hstate, and
>
> - /sys/kernel/mm/hugepages/hugepages-X/nr_hugepages/mempolicy for number of
> hugepages for a specific hstate over the set of allowed nodes.
>
> Generalize the code so that a single function handles all of these writes
> instead of duplicating the code in two different functions.
>
> This decreases the number of lines of code, but also reduces the size of
> .text by about half a percent since set_max_huge_pages() can be inlined.
>
> Signed-off-by: David Rientjes <[email protected]>
Acked-by: Davidlohr Bueso <[email protected]>
On Wed, 2 Jul 2014 17:44:46 -0700 (PDT)
David Rientjes <[email protected]> wrote:
> On Wed, 2 Jul 2014, Luiz Capitulino wrote:
>
> > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > > --- a/mm/hugetlb.c
> > > +++ b/mm/hugetlb.c
> > > @@ -1734,21 +1734,13 @@ static ssize_t nr_hugepages_show_common(struct kobject *kobj,
> > > return sprintf(buf, "%lu\n", nr_huge_pages);
> > > }
> > >
> > > -static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> > > - struct kobject *kobj, struct kobj_attribute *attr,
> > > - const char *buf, size_t len)
> > > +static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
> > > + struct hstate *h, int nid,
> > > + unsigned long count, size_t len)
> > > {
> > > int err;
> > > - int nid;
> > > - unsigned long count;
> > > - struct hstate *h;
> > > NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
> > >
> > > - err = kstrtoul(buf, 10, &count);
> > > - if (err)
> > > - goto out;
> > > -
> > > - h = kobj_to_hstate(kobj, &nid);
> > > if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> > > err = -EINVAL;
> > > goto out;
> > > @@ -1784,6 +1776,23 @@ out:
> > > return err;
> > > }
> > >
> > > +static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
> > > + struct kobject *kobj, const char *buf,
> > > + size_t len)
> > > +{
> > > + struct hstate *h;
> > > + unsigned long count;
> > > + int nid;
> > > + int err;
> > > +
> > > + err = kstrtoul(buf, 10, &count);
> > > + if (err)
> > > + return err;
> > > +
> > > + h = kobj_to_hstate(kobj, &nid);
> > > + return __nr_hugepages_store_common(obey_mempolicy, h, nid, count, len);
> > > +}
> > > +
> > > static ssize_t nr_hugepages_show(struct kobject *kobj,
> > > struct kobj_attribute *attr, char *buf)
> > > {
> > > @@ -1793,7 +1802,7 @@ static ssize_t nr_hugepages_show(struct kobject *kobj,
> > > static ssize_t nr_hugepages_store(struct kobject *kobj,
> > > struct kobj_attribute *attr, const char *buf, size_t len)
> > > {
> > > - return nr_hugepages_store_common(false, kobj, attr, buf, len);
> > > + return nr_hugepages_store_common(false, kobj, buf, len);
> > > }
> > > HSTATE_ATTR(nr_hugepages);
> > >
> > > @@ -1812,7 +1821,7 @@ static ssize_t nr_hugepages_mempolicy_show(struct kobject *kobj,
> > > static ssize_t nr_hugepages_mempolicy_store(struct kobject *kobj,
> > > struct kobj_attribute *attr, const char *buf, size_t len)
> > > {
> > > - return nr_hugepages_store_common(true, kobj, attr, buf, len);
> > > + return nr_hugepages_store_common(true, kobj, buf, len);
> > > }
> > > HSTATE_ATTR(nr_hugepages_mempolicy);
> > > #endif
> > > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > > void __user *buffer, size_t *length, loff_t *ppos)
> > > {
> > > struct hstate *h = &default_hstate;
> > > - unsigned long tmp;
> > > + unsigned long tmp = h->max_huge_pages;
> > > int ret;
> > >
> > > - if (!hugepages_supported())
> > > - return -ENOTSUPP;
> >
> > Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> > looks good to me.
> >
>
> Hmm, I think you're right but I don't think __nr_hugepages_store_common()
> is the right place: if we have a legitimate hstate for the sysfs tunables
> then we should support hugepages. I think this should be kept in
> hugetlb_sysctl_handler_common().
You seem to be right. Feel free to add if you respin:
Reviewed-by: Luiz Capitulino <[email protected]>
>
> > > -
> > > - tmp = h->max_huge_pages;
> > > -
> > > - if (write && hstate_is_gigantic(h) && !gigantic_page_supported())
> > > - return -EINVAL;
> > > -
> > > table->data = &tmp;
> > > table->maxlen = sizeof(unsigned long);
> > > ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> > > if (ret)
> > > goto out;
> > >
> > > - if (write) {
> > > - NODEMASK_ALLOC(nodemask_t, nodes_allowed,
> > > - GFP_KERNEL | __GFP_NORETRY);
> > > - if (!(obey_mempolicy &&
> > > - init_nodemask_of_mempolicy(nodes_allowed))) {
> > > - NODEMASK_FREE(nodes_allowed);
> > > - nodes_allowed = &node_states[N_MEMORY];
> > > - }
> > > - h->max_huge_pages = set_max_huge_pages(h, tmp, nodes_allowed);
> > > -
> > > - if (nodes_allowed != &node_states[N_MEMORY])
> > > - NODEMASK_FREE(nodes_allowed);
> > > - }
> > > + if (write)
> > > + ret = __nr_hugepages_store_common(obey_mempolicy, h,
> > > + NUMA_NO_NODE, tmp, *length);
> > > out:
> > > return ret;
> > > }
>
On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <[email protected]> wrote:
> > > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > > void __user *buffer, size_t *length, loff_t *ppos)
> > > {
> > > struct hstate *h = &default_hstate;
> > > - unsigned long tmp;
> > > + unsigned long tmp = h->max_huge_pages;
> > > int ret;
> > >
> > > - if (!hugepages_supported())
> > > - return -ENOTSUPP;
> >
> > Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> > looks good to me.
> >
>
> Hmm, I think you're right but I don't think __nr_hugepages_store_common()
> is the right place: if we have a legitimate hstate for the sysfs tunables
> then we should support hugepages. I think this should be kept in
> hugetlb_sysctl_handler_common().
This?
--- a/mm/hugetlb.c~mm-hugetlb-generalize-writes-to-nr_hugepages-fix
+++ a/mm/hugetlb.c
@@ -2260,6 +2260,9 @@ static int hugetlb_sysctl_handler_common
unsigned long tmp = h->max_huge_pages;
int ret;
+ if (!hugepages_supported())
+ return -ENOTSUPP;
+
table->data = &tmp;
table->maxlen = sizeof(unsigned long);
ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
_
On Tue, 8 Jul 2014 15:11:13 -0700
Andrew Morton <[email protected]> wrote:
> On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <[email protected]> wrote:
>
> > > > @@ -2248,36 +2257,18 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> > > > void __user *buffer, size_t *length, loff_t *ppos)
> > > > {
> > > > struct hstate *h = &default_hstate;
> > > > - unsigned long tmp;
> > > > + unsigned long tmp = h->max_huge_pages;
> > > > int ret;
> > > >
> > > > - if (!hugepages_supported())
> > > > - return -ENOTSUPP;
> > >
> > > Shouldn't you add this check to __nr_hugepages_store_common()? Otherwise
> > > looks good to me.
> > >
> >
> > Hmm, I think you're right but I don't think __nr_hugepages_store_common()
> > is the right place: if we have a legitimate hstate for the sysfs tunables
> > then we should support hugepages. I think this should be kept in
> > hugetlb_sysctl_handler_common().
>
> This?
Yes.
>
> --- a/mm/hugetlb.c~mm-hugetlb-generalize-writes-to-nr_hugepages-fix
> +++ a/mm/hugetlb.c
> @@ -2260,6 +2260,9 @@ static int hugetlb_sysctl_handler_common
> unsigned long tmp = h->max_huge_pages;
> int ret;
>
> + if (!hugepages_supported())
> + return -ENOTSUPP;
> +
> table->data = &tmp;
> table->maxlen = sizeof(unsigned long);
> ret = proc_doulongvec_minmax(table, write, buffer, length, ppos);
> _
>