Re: [PATCH 4/5] Add sysctl for default hstate nodes_allowed.

[Mel Gorman <mel@csn.ul.ie>]

On Tue, Jun 16, 2009 at 09:53:08AM -0400, Lee Schermerhorn wrote:
> [PATCH 4/5] add sysctl for default hstate nodes_allowed.
> 
> Against:  17may09 mmotm
> 
> This patch adds a sysctl -- /proc/sys/vm/hugepages_nodes_allowed --
> to set/query the default hstate's nodes_allowed.  I don't know
> that this patch is required, given that we have the per hstate
> controls in /sys/kernel/mm/hugepages/*. However, we've added sysctls
> for other recent hugepages controls, like nr_overcommit_hugepages,
> so I've followed that convention.
> 

Yeah, it's somewhat expected that what is in /proc/sys/vm is information
on the default hugepage size.

> Factor the formatting of the nodes_allowed mask out of nodes_allowed_show()
> for use by both that function and the hugetlb_nodes_allowed_handler().
> 
> Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp>
> 
>  include/linux/hugetlb.h |    1 +
>  kernel/sysctl.c         |    8 ++++++++
>  mm/hugetlb.c            |   43 ++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 47 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6.30-rc8-mmotm-090603-1633/include/linux/hugetlb.h
> ===================================================================
> --- linux-2.6.30-rc8-mmotm-090603-1633.orig/include/linux/hugetlb.h	2009-06-04 12:59:32.000000000 -0400
> +++ linux-2.6.30-rc8-mmotm-090603-1633/include/linux/hugetlb.h	2009-06-04 12:59:35.000000000 -0400
> @@ -22,6 +22,7 @@ void reset_vma_resv_huge_pages(struct vm
>  int hugetlb_sysctl_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *);
>  int hugetlb_overcommit_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *);
>  int hugetlb_treat_movable_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *);
> +int hugetlb_nodes_allowed_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *);
>  int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *);
>  int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int *, int, int);
>  void unmap_hugepage_range(struct vm_area_struct *,
> Index: linux-2.6.30-rc8-mmotm-090603-1633/kernel/sysctl.c
> ===================================================================
> --- linux-2.6.30-rc8-mmotm-090603-1633.orig/kernel/sysctl.c	2009-06-04 12:59:26.000000000 -0400
> +++ linux-2.6.30-rc8-mmotm-090603-1633/kernel/sysctl.c	2009-06-04 12:59:35.000000000 -0400
> @@ -1108,6 +1108,14 @@ static struct ctl_table vm_table[] = {
>  		.extra1		= (void *)&hugetlb_zero,
>  		.extra2		= (void *)&hugetlb_infinity,
>  	},
> +	{
> +		.ctl_name	= CTL_UNNUMBERED,
> +		.procname	= "hugepages_nodes_allowed",
> +		.data		= NULL,
> +		.maxlen		= sizeof(unsigned long),
> +		.mode		= 0644,
> +		.proc_handler	= &hugetlb_nodes_allowed_handler,
> +	},
>  #endif
>  	{
>  		.ctl_name	= VM_LOWMEM_RESERVE_RATIO,
> Index: linux-2.6.30-rc8-mmotm-090603-1633/mm/hugetlb.c
> ===================================================================
> --- linux-2.6.30-rc8-mmotm-090603-1633.orig/mm/hugetlb.c	2009-06-04 12:59:33.000000000 -0400
> +++ linux-2.6.30-rc8-mmotm-090603-1633/mm/hugetlb.c	2009-06-04 12:59:35.000000000 -0400
> @@ -1354,19 +1354,27 @@ static ssize_t nr_overcommit_hugepages_s
>  }
>  HSTATE_ATTR(nr_overcommit_hugepages);
>  
> -static ssize_t nodes_allowed_show(struct kobject *kobj,
> -					struct kobj_attribute *attr, char *buf)
> +static int format_hstate_nodes_allowed(struct hstate *h, char *buf,
> +					size_t buflen)
>  {
> -	struct hstate *h = kobj_to_hstate(kobj);
>  	int len = 3;
>  
>  	if (h->nodes_allowed == &node_online_map)
>  		strcpy(buf, "all");
>  	else
> -		len = nodelist_scnprintf(buf, PAGE_SIZE,
> +		len = nodelist_scnprintf(buf, buflen,
>  					*h->nodes_allowed);
> +	return len;
> +
> +}

This looks like unnecessary churn and could have been done in the earlier
patch introducing nodes_allowed_show()

> +
> +static ssize_t nodes_allowed_show(struct kobject *kobj,
> +					struct kobj_attribute *attr, char *buf)
> +{
> +	struct hstate *h = kobj_to_hstate(kobj);
> +	int len =  format_hstate_nodes_allowed(h, buf, PAGE_SIZE);
>  
> -	if (len)
> +	if (len && (len +1) < PAGE_SIZE)
>  		buf[len++] = '\n';
>  
>  	return len;
> @@ -1684,6 +1692,31 @@ int hugetlb_overcommit_handler(struct ct
>  	return 0;
>  }
>  
> +#define NODES_ALLOWED_MAX 64
> +int hugetlb_nodes_allowed_handler(struct ctl_table *table, int write,
> +			struct file *file, void __user *buffer,
> +			size_t *length, loff_t *ppos)
> +{
> +	struct hstate *h = &default_hstate;
> +	int ret = 0;
> +
> +	if (write) {
> +		(void)set_hstate_nodes_allowed(h, buffer, 1);
> +	} else {
> +		char buf[NODES_ALLOWED_MAX];
> +		struct ctl_table tbl = {
> +			.data = buf,
> +			.maxlen = NODES_ALLOWED_MAX,
> +		};
> +		int len =  format_hstate_nodes_allowed(h, buf, sizeof(buf));
> +
> +		if (len)
> +			ret = proc_dostring(&tbl, write, file, buffer,
> +						 length, ppos);
> +	}
> +	return ret;
> +}
> +
>  #endif /* CONFIG_SYSCTL */
>  
>  void hugetlb_report_meminfo(struct seq_file *m)
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

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