Re: [PATCH v6 3/4] drivers/base/node.c: use bin_attribute to break the size limitation of cpumap ABI

[Andy Shevchenko]

On Fri, Jul 09, 2021 at 07:55:43PM +1200, Barry Song wrote:
> From: Tian Tao <tiantao6@xxxxxxxxxxxxx>
> 
> Reading sys/devices/system/cpu/cpuX/nodeX/ returns cpumap and cpulist.

/sys

> However, the size of this file is limited to PAGE_SIZE because of the
> limitation for sysfs attribute.
> 
> This patch moves to use bin_attribute to extend the ABI to be more
> than one page so that cpumap bitmask and list won't be potentially
> trimmed.
> 
> Signed-off-by: Tian Tao <tiantao6@xxxxxxxxxxxxx>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> Signed-off-by: Barry Song <song.bao.hua@xxxxxxxxxxxxx>
> ---
>  -v6: remove an extra blank line according to Andy Shevchenko's comment;
> 
>  drivers/base/node.c | 51 +++++++++++++++++++++++++++++----------------
>  1 file changed, 33 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 4a4ae868ad9f..89a72aba72a3 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -27,42 +27,44 @@ static struct bus_type node_subsys = {
>  };
>  
>  
> -static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
> +static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf,
> +				loff_t off, size_t count)
>  {
>  	ssize_t n;
>  	cpumask_var_t mask;
>  	struct node *node_dev = to_node(dev);
>  
> -	/* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> -	BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
> -
>  	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
>  		return 0;
>  
>  	cpumask_and(mask, cpumask_of_node(node_dev->dev.id), cpu_online_mask);
> -	n = cpumap_print_to_pagebuf(list, buf, mask);
> +	n = cpumap_print_to_buf(list, buf, mask, off, count);
>  	free_cpumask_var(mask);
>  
>  	return n;
>  }
>  
> -static inline ssize_t cpumap_show(struct device *dev,
> -				  struct device_attribute *attr,
> -				  char *buf)
> +static inline ssize_t cpumap_read(struct file *file, struct kobject *kobj,
> +				  struct bin_attribute *attr, char *buf,
> +				  loff_t off, size_t count)
>  {
> -	return node_read_cpumap(dev, false, buf);
> +	struct device *dev = kobj_to_dev(kobj);
> +
> +	return node_read_cpumap(dev, false, buf, off, count);
>  }
>  
> -static DEVICE_ATTR_RO(cpumap);
> +static BIN_ATTR_RO(cpumap, 0);
>  
> -static inline ssize_t cpulist_show(struct device *dev,
> -				   struct device_attribute *attr,
> -				   char *buf)
> +static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
> +				   struct bin_attribute *attr, char *buf,
> +				   loff_t off, size_t count)
>  {
> -	return node_read_cpumap(dev, true, buf);
> +	struct device *dev = kobj_to_dev(kobj);
> +
> +	return node_read_cpumap(dev, true, buf, off, count);
>  }
>  
> -static DEVICE_ATTR_RO(cpulist);
> +static BIN_ATTR_RO(cpulist, 0);
>  
>  /**
>   * struct node_access_nodes - Access class device to hold user visible
> @@ -557,15 +559,28 @@ static ssize_t node_read_distance(struct device *dev,
>  static DEVICE_ATTR(distance, 0444, node_read_distance, NULL);
>  
>  static struct attribute *node_dev_attrs[] = {
> -	&dev_attr_cpumap.attr,
> -	&dev_attr_cpulist.attr,
>  	&dev_attr_meminfo.attr,
>  	&dev_attr_numastat.attr,
>  	&dev_attr_distance.attr,
>  	&dev_attr_vmstat.attr,
>  	NULL
>  };
> -ATTRIBUTE_GROUPS(node_dev);
> +
> +static struct bin_attribute *node_dev_bin_attrs[] = {
> +	&bin_attr_cpumap,
> +	&bin_attr_cpulist,
> +	NULL
> +};
> +
> +static const struct attribute_group node_dev_group = {
> +	.attrs = node_dev_attrs,
> +	.bin_attrs = node_dev_bin_attrs
> +};
> +
> +static const struct attribute_group *node_dev_groups[] = {
> +	&node_dev_group,
> +	NULL
> +};
>  
>  #ifdef CONFIG_HUGETLBFS
>  /*
> -- 
> 2.25.1
> 

-- 
With Best Regards,
Andy Shevchenko