* [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity
2014-06-30 23:57 [patch] mm, hugetlb: generalize writes to nr_hugepages David Rientjes
@ 2014-07-01 0:46 ` David Rientjes
2014-07-01 14:05 ` Naoya Horiguchi
2014-07-02 21:38 ` Luiz Capitulino
2014-07-01 13:09 ` [patch] mm, hugetlb: generalize writes to nr_hugepages Naoya Horiguchi
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: David Rientjes @ 2014-07-01 0:46 UTC (permalink / raw)
To: Andrew Morton
Cc: Joonsoo Kim, Naoya Horiguchi, Luiz Capitulino, Kirill A. Shutemov,
linux-kernel, linux-mm
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 <rientjes@google.com>
---
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 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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity
2014-07-01 0:46 ` [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity David Rientjes
@ 2014-07-01 14:05 ` Naoya Horiguchi
2014-07-02 21:38 ` Luiz Capitulino
1 sibling, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2014-07-01 14:05 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Luiz Capitulino, Kirill A. Shutemov,
linux-kernel, linux-mm
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 <rientjes@google.com>
Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> 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 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>
>
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity
2014-07-01 0:46 ` [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity David Rientjes
2014-07-01 14:05 ` Naoya Horiguchi
@ 2014-07-02 21:38 ` Luiz Capitulino
1 sibling, 0 replies; 11+ messages in thread
From: Luiz Capitulino @ 2014-07-02 21:38 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
On Mon, 30 Jun 2014 17:46:35 -0700 (PDT)
David Rientjes <rientjes@google.com> 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 <rientjes@google.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> 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 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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-06-30 23:57 [patch] mm, hugetlb: generalize writes to nr_hugepages David Rientjes
2014-07-01 0:46 ` [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity David Rientjes
@ 2014-07-01 13:09 ` Naoya Horiguchi
2014-07-02 21:25 ` Luiz Capitulino
2014-07-03 2:04 ` Davidlohr Bueso
3 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2014-07-01 13:09 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Luiz Capitulino, Kirill A. Shutemov,
linux-kernel, linux-mm
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 <rientjes@google.com>
Looks good to me.
Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> 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 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>
>
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-06-30 23:57 [patch] mm, hugetlb: generalize writes to nr_hugepages David Rientjes
2014-07-01 0:46 ` [patch] mm, hugetlb: remove hugetlb_zero and hugetlb_infinity David Rientjes
2014-07-01 13:09 ` [patch] mm, hugetlb: generalize writes to nr_hugepages Naoya Horiguchi
@ 2014-07-02 21:25 ` Luiz Capitulino
2014-07-03 0:44 ` David Rientjes
2014-07-03 2:04 ` Davidlohr Bueso
3 siblings, 1 reply; 11+ messages in thread
From: Luiz Capitulino @ 2014-07-02 21:25 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
On Mon, 30 Jun 2014 16:57:06 -0700 (PDT)
David Rientjes <rientjes@google.com> 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 <rientjes@google.com>
> ---
> 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;
> }
>
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-07-02 21:25 ` Luiz Capitulino
@ 2014-07-03 0:44 ` David Rientjes
2014-07-03 3:37 ` Luiz Capitulino
2014-07-08 22:11 ` Andrew Morton
0 siblings, 2 replies; 11+ messages in thread
From: David Rientjes @ 2014-07-03 0:44 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Andrew Morton, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
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;
> > }
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-07-03 0:44 ` David Rientjes
@ 2014-07-03 3:37 ` Luiz Capitulino
2014-07-08 22:11 ` Andrew Morton
1 sibling, 0 replies; 11+ messages in thread
From: Luiz Capitulino @ 2014-07-03 3:37 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
On Wed, 2 Jul 2014 17:44:46 -0700 (PDT)
David Rientjes <rientjes@google.com> 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 <lcapitulino@redhat.com>
>
> > > -
> > > - 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 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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-07-03 0:44 ` David Rientjes
2014-07-03 3:37 ` Luiz Capitulino
@ 2014-07-08 22:11 ` Andrew Morton
2014-07-09 13:31 ` Luiz Capitulino
1 sibling, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2014-07-08 22:11 UTC (permalink / raw)
To: David Rientjes
Cc: Luiz Capitulino, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <rientjes@google.com> 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);
_
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-07-08 22:11 ` Andrew Morton
@ 2014-07-09 13:31 ` Luiz Capitulino
0 siblings, 0 replies; 11+ messages in thread
From: Luiz Capitulino @ 2014-07-09 13:31 UTC (permalink / raw)
To: Andrew Morton
Cc: David Rientjes, Joonsoo Kim, Naoya Horiguchi, Kirill A. Shutemov,
linux-kernel, linux-mm
On Tue, 8 Jul 2014 15:11:13 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 2 Jul 2014 17:44:46 -0700 (PDT) David Rientjes <rientjes@google.com> 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);
> _
>
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] mm, hugetlb: generalize writes to nr_hugepages
2014-06-30 23:57 [patch] mm, hugetlb: generalize writes to nr_hugepages David Rientjes
` (2 preceding siblings ...)
2014-07-02 21:25 ` Luiz Capitulino
@ 2014-07-03 2:04 ` Davidlohr Bueso
3 siblings, 0 replies; 11+ messages in thread
From: Davidlohr Bueso @ 2014-07-03 2:04 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Joonsoo Kim, Naoya Horiguchi, Luiz Capitulino,
Kirill A. Shutemov, linux-kernel, linux-mm
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 <rientjes@google.com>
Acked-by: Davidlohr Bueso <davidlohr@hp.com>
--
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>
^ permalink raw reply [flat|nested] 11+ messages in thread