* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-03 17:00 ` [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function Sudeep Holla
@ 2014-09-03 17:05 ` Bjorn Helgaas
2014-09-03 17:09 ` Sudeep Holla
2014-09-03 21:46 ` Rafael J. Wysocki
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2014-09-03 17:05 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Heiko Carstens, Lorenzo Pieralisi, Greg Kroah-Hartman,
Peter Zijlstra, Rafael J. Wysocki, x86, linux-acpi, linux-pci
On Wed, Sep 3, 2014 at 11:00 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
> terminated buffer with newline.
>
> This patch creates a new helper function cpumap_copy_to_buf in
> cpumask.h and consolidates most of them using this new helper.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: x86@kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
Acked-by: Bjorn Helgaas <bhelgaas@google.com> (drivers/pci part)
I assume you'll merge this via a non-PCI tree.
> ---
> arch/x86/kernel/cpu/perf_event_amd_iommu.c | 5 +---
> arch/x86/kernel/cpu/perf_event_amd_uncore.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_rapl.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 6 +----
> drivers/acpi/acpi_pad.c | 6 +----
> drivers/base/cpu.c | 5 +---
> drivers/base/node.c | 14 +++-------
> drivers/base/topology.c | 22 ++-------------
> drivers/pci/pci-sysfs.c | 39 +++++++--------------------
> include/linux/cpumask.h | 27 +++++++++++++++++++
> 10 files changed, 49 insertions(+), 87 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_iommu.c b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> index 639d1289b1ba..61a670de0ef2 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> @@ -130,10 +130,7 @@ static ssize_t _iommu_cpumask_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &iommu_cpumask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &iommu_cpumask, buf);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, _iommu_cpumask_show, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_uncore.c b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> index 30790d798e6b..d96a37ed8570 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> @@ -219,7 +219,6 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n;
> cpumask_t *active_mask;
> struct pmu *pmu = dev_get_drvdata(dev);
>
> @@ -230,10 +229,7 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> else
> return 0;
>
> - n = cpulist_scnprintf(buf, PAGE_SIZE - 2, active_mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, active_mask, buf);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, amd_uncore_attr_show_cpumask, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_rapl.c b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> index 619f7699487a..8a673e138e2e 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> @@ -365,11 +365,7 @@ static void rapl_pmu_event_read(struct perf_event *event)
> static ssize_t rapl_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &rapl_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &rapl_cpu_mask, buf);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0939f86f543d..a0b810951743 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -3617,11 +3617,7 @@ static int uncore_pmu_event_init(struct perf_event *event)
> static ssize_t uncore_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &uncore_cpu_mask, buf);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
> diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> index f148a0580e04..0dbe52ed6a0d 100644
> --- a/drivers/acpi/acpi_pad.c
> +++ b/drivers/acpi/acpi_pad.c
> @@ -350,11 +350,7 @@ static ssize_t acpi_pad_idlecpus_store(struct device *dev,
> static ssize_t acpi_pad_idlecpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = 0;
> - n = cpumask_scnprintf(buf, PAGE_SIZE-2, to_cpumask(pad_busy_cpus_bits));
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(false, to_cpumask(pad_busy_cpus_bits), buf);
> }
> static DEVICE_ATTR(idlecpus, S_IRUGO|S_IWUSR,
> acpi_pad_idlecpus_show,
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc5297d..277a9cfa9040 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -207,11 +207,8 @@ static ssize_t show_cpus_attr(struct device *dev,
> char *buf)
> {
> struct cpu_attr *ca = container_of(attr, struct cpu_attr, attr);
> - int n = cpulist_scnprintf(buf, PAGE_SIZE-2, *(ca->map));
>
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, *ca->map, buf);
> }
>
> #define _CPU_ATTR(name, map) \
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index c6d3ae05f1ca..6e58ad566c13 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -25,32 +25,26 @@ static struct bus_type node_subsys = {
> };
>
>
> -static ssize_t node_read_cpumap(struct device *dev, int type, char *buf)
> +static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
> {
> struct node *node_dev = to_node(dev);
> const struct cpumask *mask = cpumask_of_node(node_dev->dev.id);
> - int len;
>
> /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
>
> - len = type?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask);
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_copy_to_buf(list, mask, buf);
> }
>
> static inline ssize_t node_read_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 0, buf);
> + return node_read_cpumap(dev, false, buf);
> }
> static inline ssize_t node_read_cpulist(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 1, buf);
> + return node_read_cpumap(dev, true, buf);
> }
>
> static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask, NULL);
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index be7c1fb7c0c9..39b294bd769b 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -42,29 +42,11 @@ static ssize_t show_##name(struct device *dev, \
> return sprintf(buf, "%d\n", topology_##name(dev->id)); \
> }
>
> -#if defined(topology_thread_cpumask) || defined(topology_core_cpumask) || \
> - defined(topology_book_cpumask)
> -static ssize_t show_cpumap(int type, const struct cpumask *mask, char *buf)
> -{
> - ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
> - int n = 0;
> -
> - if (len > 1) {
> - n = type?
> - cpulist_scnprintf(buf, len-2, mask) :
> - cpumask_scnprintf(buf, len-2, mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - }
> - return n;
> -}
> -#endif
> -
> #define define_siblings_show_map(name) \
> static ssize_t show_##name(struct device *dev, \
> struct device_attribute *attr, char *buf) \
> { \
> - return show_cpumap(0, topology_##name(dev->id), buf); \
> + return cpumap_copy_to_buf(false, topology_##name(dev->id), buf);\
> }
>
> #define define_siblings_show_list(name) \
> @@ -72,7 +54,7 @@ static ssize_t show_##name##_list(struct device *dev, \
> struct device_attribute *attr, \
> char *buf) \
> { \
> - return show_cpumap(1, topology_##name(dev->id), buf); \
> + return cpumap_copy_to_buf(true, topology_##name(dev->id), buf); \
> }
>
> #define define_siblings_show_func(name) \
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9ff0a901ecf7..aa0f1359ef30 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -77,11 +77,10 @@ static ssize_t broken_parity_status_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(broken_parity_status);
>
> -static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> +static ssize_t pci_dev_show_local_cpu(struct device *dev, bool list,
> struct device_attribute *attr, char *buf)
> {
> const struct cpumask *mask;
> - int len;
>
> #ifdef CONFIG_NUMA
> mask = (dev_to_node(dev) == -1) ? cpu_online_mask :
> @@ -89,59 +88,41 @@ static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> #else
> mask = cpumask_of_pcibus(to_pci_dev(dev)->bus);
> #endif
> - len = type ?
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask);
> -
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_copy_to_buf(list, mask, buf);
> }
>
> static ssize_t local_cpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 1, attr, buf);
> + return pci_dev_show_local_cpu(dev, false, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpus);
>
> static ssize_t local_cpulist_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 0, attr, buf);
> + return pci_dev_show_local_cpu(dev, true, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpulist);
>
> /*
> * PCI Bus Class Devices
> */
> -static ssize_t pci_bus_show_cpuaffinity(struct device *dev, int type,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - int ret;
> - const struct cpumask *cpumask;
> -
> - cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> - ret = type ?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, cpumask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, cpumask);
> - buf[ret++] = '\n';
> - buf[ret] = '\0';
> - return ret;
> -}
> -
> static ssize_t cpuaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 0, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_copy_to_buf(false, cpumask, buf);
> }
> static DEVICE_ATTR_RO(cpuaffinity);
>
> static ssize_t cpulistaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 1, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_copy_to_buf(true, cpumask, buf);
> }
> static DEVICE_ATTR_RO(cpulistaffinity);
>
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 2997af6d2ccd..26d8348292dd 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -11,6 +11,8 @@
> #include <linux/bitmap.h>
> #include <linux/bug.h>
>
> +#include <asm/page.h>
> +
> typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
>
> /**
> @@ -792,6 +794,31 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
> }
> #endif /* NR_CPUS > BITS_PER_LONG */
>
> +/**
> + * cpumap_copy_to_buf - copies the cpumask into the buffer either
> + * as comma-separated list of cpus or hex values of cpumask
> + * @list: indicates whether the cpumap must be list
> + * @mask: the cpumask to copy
> + * @buf: the buffer to copy into
> + *
> + * Returns the length of the (null-terminated) @buf string, zero if
> + * nothing is copied.
> + */
> +static inline ssize_t
> +cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
> +{
> + ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
> + int n = 0;
> +
> + if (len > 1) {
> + n = list ? cpulist_scnprintf(buf, len, mask) :
> + cpumask_scnprintf(buf, len, mask);
> + buf[n++] = '\n';
> + buf[n] = '\0';
> + }
> + return n;
> +}
> +
> /*
> *
> * From here down, all obsolete. Use cpumask_ variants!
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-03 17:05 ` Bjorn Helgaas
@ 2014-09-03 17:09 ` Sudeep Holla
0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2014-09-03 17:09 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Sudeep Holla, LKML, Heiko Carstens, Lorenzo Pieralisi,
Greg Kroah-Hartman, Peter Zijlstra, Rafael J. Wysocki, x86,
linux-acpi, linux-pci
Hi Bjorn,
On 03/09/14 18:05, Bjorn Helgaas wrote:
> On Wed, Sep 3, 2014 at 11:00 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> From: Sudeep Holla <sudeep.holla@arm.com>
>>
>> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
>> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
>> terminated buffer with newline.
>>
>> This patch creates a new helper function cpumap_copy_to_buf in
>> cpumask.h and consolidates most of them using this new helper.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: x86@kernel.org
>> Cc: linux-acpi@vger.kernel.org
>> Cc: linux-pci@vger.kernel.org
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com> (drivers/pci part)
>
Thanks.
> I assume you'll merge this via a non-PCI tree.
>
Yes, since it's widespread change, I prefer it to go through Greg's tree.
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-03 17:00 ` [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function Sudeep Holla
2014-09-03 17:05 ` Bjorn Helgaas
@ 2014-09-03 21:46 ` Rafael J. Wysocki
2014-09-04 6:20 ` Peter Zijlstra
2014-09-04 15:46 ` [PATCH v4 01/11 UPDATE] " Sudeep Holla
3 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2014-09-03 21:46 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Heiko Carstens, Lorenzo Pieralisi, Greg Kroah-Hartman,
Peter Zijlstra, Bjorn Helgaas, x86, linux-acpi, linux-pci
On Wednesday, September 03, 2014 06:00:07 PM Sudeep Holla wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
> terminated buffer with newline.
>
> This patch creates a new helper function cpumap_copy_to_buf in
> cpumask.h and consolidates most of them using this new helper.
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> for the ACPI PAD
part.
Thanks!
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: x86@kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
> arch/x86/kernel/cpu/perf_event_amd_iommu.c | 5 +---
> arch/x86/kernel/cpu/perf_event_amd_uncore.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_rapl.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 6 +----
> drivers/acpi/acpi_pad.c | 6 +----
> drivers/base/cpu.c | 5 +---
> drivers/base/node.c | 14 +++-------
> drivers/base/topology.c | 22 ++-------------
> drivers/pci/pci-sysfs.c | 39 +++++++--------------------
> include/linux/cpumask.h | 27 +++++++++++++++++++
> 10 files changed, 49 insertions(+), 87 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_iommu.c b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> index 639d1289b1ba..61a670de0ef2 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> @@ -130,10 +130,7 @@ static ssize_t _iommu_cpumask_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &iommu_cpumask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &iommu_cpumask, buf);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, _iommu_cpumask_show, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_uncore.c b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> index 30790d798e6b..d96a37ed8570 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> @@ -219,7 +219,6 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n;
> cpumask_t *active_mask;
> struct pmu *pmu = dev_get_drvdata(dev);
>
> @@ -230,10 +229,7 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> else
> return 0;
>
> - n = cpulist_scnprintf(buf, PAGE_SIZE - 2, active_mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, active_mask, buf);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, amd_uncore_attr_show_cpumask, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_rapl.c b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> index 619f7699487a..8a673e138e2e 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> @@ -365,11 +365,7 @@ static void rapl_pmu_event_read(struct perf_event *event)
> static ssize_t rapl_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &rapl_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &rapl_cpu_mask, buf);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0939f86f543d..a0b810951743 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -3617,11 +3617,7 @@ static int uncore_pmu_event_init(struct perf_event *event)
> static ssize_t uncore_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, &uncore_cpu_mask, buf);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
> diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> index f148a0580e04..0dbe52ed6a0d 100644
> --- a/drivers/acpi/acpi_pad.c
> +++ b/drivers/acpi/acpi_pad.c
> @@ -350,11 +350,7 @@ static ssize_t acpi_pad_idlecpus_store(struct device *dev,
> static ssize_t acpi_pad_idlecpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = 0;
> - n = cpumask_scnprintf(buf, PAGE_SIZE-2, to_cpumask(pad_busy_cpus_bits));
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(false, to_cpumask(pad_busy_cpus_bits), buf);
> }
> static DEVICE_ATTR(idlecpus, S_IRUGO|S_IWUSR,
> acpi_pad_idlecpus_show,
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc5297d..277a9cfa9040 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -207,11 +207,8 @@ static ssize_t show_cpus_attr(struct device *dev,
> char *buf)
> {
> struct cpu_attr *ca = container_of(attr, struct cpu_attr, attr);
> - int n = cpulist_scnprintf(buf, PAGE_SIZE-2, *(ca->map));
>
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_copy_to_buf(true, *ca->map, buf);
> }
>
> #define _CPU_ATTR(name, map) \
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index c6d3ae05f1ca..6e58ad566c13 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -25,32 +25,26 @@ static struct bus_type node_subsys = {
> };
>
>
> -static ssize_t node_read_cpumap(struct device *dev, int type, char *buf)
> +static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
> {
> struct node *node_dev = to_node(dev);
> const struct cpumask *mask = cpumask_of_node(node_dev->dev.id);
> - int len;
>
> /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
>
> - len = type?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask);
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_copy_to_buf(list, mask, buf);
> }
>
> static inline ssize_t node_read_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 0, buf);
> + return node_read_cpumap(dev, false, buf);
> }
> static inline ssize_t node_read_cpulist(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 1, buf);
> + return node_read_cpumap(dev, true, buf);
> }
>
> static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask, NULL);
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index be7c1fb7c0c9..39b294bd769b 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -42,29 +42,11 @@ static ssize_t show_##name(struct device *dev, \
> return sprintf(buf, "%d\n", topology_##name(dev->id)); \
> }
>
> -#if defined(topology_thread_cpumask) || defined(topology_core_cpumask) || \
> - defined(topology_book_cpumask)
> -static ssize_t show_cpumap(int type, const struct cpumask *mask, char *buf)
> -{
> - ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
> - int n = 0;
> -
> - if (len > 1) {
> - n = type?
> - cpulist_scnprintf(buf, len-2, mask) :
> - cpumask_scnprintf(buf, len-2, mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - }
> - return n;
> -}
> -#endif
> -
> #define define_siblings_show_map(name) \
> static ssize_t show_##name(struct device *dev, \
> struct device_attribute *attr, char *buf) \
> { \
> - return show_cpumap(0, topology_##name(dev->id), buf); \
> + return cpumap_copy_to_buf(false, topology_##name(dev->id), buf);\
> }
>
> #define define_siblings_show_list(name) \
> @@ -72,7 +54,7 @@ static ssize_t show_##name##_list(struct device *dev, \
> struct device_attribute *attr, \
> char *buf) \
> { \
> - return show_cpumap(1, topology_##name(dev->id), buf); \
> + return cpumap_copy_to_buf(true, topology_##name(dev->id), buf); \
> }
>
> #define define_siblings_show_func(name) \
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9ff0a901ecf7..aa0f1359ef30 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -77,11 +77,10 @@ static ssize_t broken_parity_status_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(broken_parity_status);
>
> -static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> +static ssize_t pci_dev_show_local_cpu(struct device *dev, bool list,
> struct device_attribute *attr, char *buf)
> {
> const struct cpumask *mask;
> - int len;
>
> #ifdef CONFIG_NUMA
> mask = (dev_to_node(dev) == -1) ? cpu_online_mask :
> @@ -89,59 +88,41 @@ static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> #else
> mask = cpumask_of_pcibus(to_pci_dev(dev)->bus);
> #endif
> - len = type ?
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask);
> -
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_copy_to_buf(list, mask, buf);
> }
>
> static ssize_t local_cpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 1, attr, buf);
> + return pci_dev_show_local_cpu(dev, false, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpus);
>
> static ssize_t local_cpulist_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 0, attr, buf);
> + return pci_dev_show_local_cpu(dev, true, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpulist);
>
> /*
> * PCI Bus Class Devices
> */
> -static ssize_t pci_bus_show_cpuaffinity(struct device *dev, int type,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - int ret;
> - const struct cpumask *cpumask;
> -
> - cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> - ret = type ?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, cpumask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, cpumask);
> - buf[ret++] = '\n';
> - buf[ret] = '\0';
> - return ret;
> -}
> -
> static ssize_t cpuaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 0, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_copy_to_buf(false, cpumask, buf);
> }
> static DEVICE_ATTR_RO(cpuaffinity);
>
> static ssize_t cpulistaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 1, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_copy_to_buf(true, cpumask, buf);
> }
> static DEVICE_ATTR_RO(cpulistaffinity);
>
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 2997af6d2ccd..26d8348292dd 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -11,6 +11,8 @@
> #include <linux/bitmap.h>
> #include <linux/bug.h>
>
> +#include <asm/page.h>
> +
> typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
>
> /**
> @@ -792,6 +794,31 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
> }
> #endif /* NR_CPUS > BITS_PER_LONG */
>
> +/**
> + * cpumap_copy_to_buf - copies the cpumask into the buffer either
> + * as comma-separated list of cpus or hex values of cpumask
> + * @list: indicates whether the cpumap must be list
> + * @mask: the cpumask to copy
> + * @buf: the buffer to copy into
> + *
> + * Returns the length of the (null-terminated) @buf string, zero if
> + * nothing is copied.
> + */
> +static inline ssize_t
> +cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
> +{
> + ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
> + int n = 0;
> +
> + if (len > 1) {
> + n = list ? cpulist_scnprintf(buf, len, mask) :
> + cpumask_scnprintf(buf, len, mask);
> + buf[n++] = '\n';
> + buf[n] = '\0';
> + }
> + return n;
> +}
> +
> /*
> *
> * From here down, all obsolete. Use cpumask_ variants!
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-03 17:00 ` [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function Sudeep Holla
2014-09-03 17:05 ` Bjorn Helgaas
2014-09-03 21:46 ` Rafael J. Wysocki
@ 2014-09-04 6:20 ` Peter Zijlstra
2014-09-04 9:03 ` Sudeep Holla
2014-09-04 15:46 ` [PATCH v4 01/11 UPDATE] " Sudeep Holla
3 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2014-09-04 6:20 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Heiko Carstens, Lorenzo Pieralisi, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, x86, linux-acpi, linux-pci
On Wed, Sep 03, 2014 at 06:00:07PM +0100, Sudeep Holla wrote:
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 2997af6d2ccd..26d8348292dd 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -11,6 +11,8 @@
> #include <linux/bitmap.h>
> #include <linux/bug.h>
>
> +#include <asm/page.h>
> +
> typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
>
> /**
> @@ -792,6 +794,31 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
> }
> #endif /* NR_CPUS > BITS_PER_LONG */
>
> +/**
> + * cpumap_copy_to_buf - copies the cpumask into the buffer either
> + * as comma-separated list of cpus or hex values of cpumask
> + * @list: indicates whether the cpumap must be list
> + * @mask: the cpumask to copy
> + * @buf: the buffer to copy into
> + *
> + * Returns the length of the (null-terminated) @buf string, zero if
> + * nothing is copied.
> + */
> +static inline ssize_t
> +cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
> +{
> + ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
> + int n = 0;
> +
> + if (len > 1) {
> + n = list ? cpulist_scnprintf(buf, len, mask) :
> + cpumask_scnprintf(buf, len, mask);
> + buf[n++] = '\n';
> + buf[n] = '\0';
> + }
> + return n;
> +}
> +
Does that really make sense as an inline?
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-04 6:20 ` Peter Zijlstra
@ 2014-09-04 9:03 ` Sudeep Holla
2014-09-04 9:21 ` Peter Zijlstra
0 siblings, 1 reply; 14+ messages in thread
From: Sudeep Holla @ 2014-09-04 9:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sudeep Holla, LKML, Heiko Carstens, Lorenzo Pieralisi,
Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas, x86,
linux-acpi, linux-pci
Hi Peter,
On 04/09/14 07:20, Peter Zijlstra wrote:
> On Wed, Sep 03, 2014 at 06:00:07PM +0100, Sudeep Holla wrote:
>> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
>> index 2997af6d2ccd..26d8348292dd 100644
>> --- a/include/linux/cpumask.h
>> +++ b/include/linux/cpumask.h
>> @@ -11,6 +11,8 @@
>> #include <linux/bitmap.h>
>> #include <linux/bug.h>
>>
>> +#include <asm/page.h>
>> +
>> typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;
>>
>> /**
>> @@ -792,6 +794,31 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
>> }
>> #endif /* NR_CPUS > BITS_PER_LONG */
>>
>> +/**
>> + * cpumap_copy_to_buf - copies the cpumask into the buffer either
>> + * as comma-separated list of cpus or hex values of cpumask
>> + * @list: indicates whether the cpumap must be list
>> + * @mask: the cpumask to copy
>> + * @buf: the buffer to copy into
>> + *
>> + * Returns the length of the (null-terminated) @buf string, zero if
>> + * nothing is copied.
>> + */
>> +static inline ssize_t
>> +cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
>> +{
>> + ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
>> + int n = 0;
>> +
>> + if (len > 1) {
>> + n = list ? cpulist_scnprintf(buf, len, mask) :
>> + cpumask_scnprintf(buf, len, mask);
>> + buf[n++] = '\n';
>> + buf[n] = '\0';
>> + }
>> + return n;
>> +}
>> +
>
> Does that really make sense as an inline?
>
If I don't, every file including this header generates "warning:
‘cpumap_copy_to_buf’ defined but not used". Alternatively I can just
declare here and define it elsewhere but I could not find more apt
place than this. Any suggestions ? kernel/cpu.c ?
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-04 9:03 ` Sudeep Holla
@ 2014-09-04 9:21 ` Peter Zijlstra
2014-09-04 10:43 ` Sudeep Holla
0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2014-09-04 9:21 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Heiko Carstens, Lorenzo Pieralisi, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, x86, linux-acpi, linux-pci
On Thu, Sep 04, 2014 at 10:03:25AM +0100, Sudeep Holla wrote:
> If I don't, every file including this header generates "warning:
> ‘cpumap_copy_to_buf’ defined but not used". Alternatively I can just
> declare here and define it elsewhere but I could not find more apt
> place than this. Any suggestions ? kernel/cpu.c ?
Sure the tail of cpu.c seems to have various other cpumask gunk in
already so it fits there. The other alternative would've been
lib/bitmap.c since that's where the various bitmap printf
implementations live.
You could make it bitmap_copy_to_buf() I suppose, and then have a
cpumask inline wrapper, to be fully consistent with the rest of the
implementations.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-04 9:21 ` Peter Zijlstra
@ 2014-09-04 10:43 ` Sudeep Holla
2014-09-04 11:25 ` Peter Zijlstra
0 siblings, 1 reply; 14+ messages in thread
From: Sudeep Holla @ 2014-09-04 10:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sudeep Holla, LKML, Heiko Carstens, Lorenzo Pieralisi,
Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas, x86,
linux-acpi, linux-pci
Hi Peter,
On 04/09/14 10:21, Peter Zijlstra wrote:
> On Thu, Sep 04, 2014 at 10:03:25AM +0100, Sudeep Holla wrote:
>> If I don't, every file including this header generates "warning:
>> ‘cpumap_copy_to_buf’ defined but not used". Alternatively I can just
>> declare here and define it elsewhere but I could not find more apt
>> place than this. Any suggestions ? kernel/cpu.c ?
>
> Sure the tail of cpu.c seems to have various other cpumask gunk in
> already so it fits there. The other alternative would've been
> lib/bitmap.c since that's where the various bitmap printf
> implementations live.
>
> You could make it bitmap_copy_to_buf() I suppose, and then have a
> cpumask inline wrapper, to be fully consistent with the rest of the
> implementations.
>
I liked the second approach, how about patch below ?
If will munge it into original patch.
--->8
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e1c8d080c427..34586d386c1f 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -60,6 +60,7 @@
* bitmap_find_free_region(bitmap, bits, order) Find and
allocate bit region
* bitmap_release_region(bitmap, pos, order) Free specified bit region
* bitmap_allocate_region(bitmap, pos, order) Allocate specified bit
region
+ * bitmap_copy_to_pagebuf(list, src, buf) Print bitmap src as
list/hex to buf
*/
/*
@@ -145,6 +146,8 @@ extern void bitmap_release_region(unsigned long
*bitmap, unsigned int pos, int o
extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int
pos, int order);
extern void bitmap_copy_le(void *dst, const unsigned long *src, int
nbits);
extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int
bits);
+extern int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp,
+ char *buf);
#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
#define BITMAP_LAST_WORD_MASK(nbits) \
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 26d8348292dd..5b3bd163c5a0 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -807,16 +807,7 @@ static inline const struct cpumask
*get_cpu_mask(unsigned int cpu)
static inline ssize_t
cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
{
- ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
- int n = 0;
-
- if (len > 1) {
- n = list ? cpulist_scnprintf(buf, len, mask) :
- cpumask_scnprintf(buf, len, mask);
- buf[n++] = '\n';
- buf[n] = '\0';
- }
- return n;
+ return bitmap_copy_to_pagebuf(list, cpumask_bits(mask), buf);
}
sudeep@e103737-lin:~/work/power/kernel$ git diff > /tmp/log
sudeep@e103737-lin:~/work/power/kernel$ cat /tmp/log
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e1c8d080c427..34586d386c1f 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -60,6 +60,7 @@
* bitmap_find_free_region(bitmap, bits, order) Find and allocate bit
region
* bitmap_release_region(bitmap, pos, order) Free specified bit region
* bitmap_allocate_region(bitmap, pos, order) Allocate specified bit
region
+ * bitmap_copy_to_pagebuf(list, src, buf) Print bitmap src as list/hex
to buf
*/
/*
@@ -145,6 +146,8 @@ extern void bitmap_release_region(unsigned long
*bitmap, unsigned int pos, int o
extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int
pos, int order);
extern void bitmap_copy_le(void *dst, const unsigned long *src, int
nbits);
extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int
bits);
+extern int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp,
+ char *buf);
#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
#define BITMAP_LAST_WORD_MASK(nbits) \
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 26d8348292dd..5b3bd163c5a0 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -807,16 +807,7 @@ static inline const struct cpumask
*get_cpu_mask(unsigned int cpu)
static inline ssize_t
cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
{
- ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
- int n = 0;
-
- if (len > 1) {
- n = list ? cpulist_scnprintf(buf, len, mask) :
- cpumask_scnprintf(buf, len, mask);
- buf[n++] = '\n';
- buf[n] = '\0';
- }
- return n;
+ return bitmap_copy_to_pagebuf(list, cpumask_bits(mask), buf);
}
/*
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 1e031f2c9aba..b23adef04d7e 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -580,6 +580,32 @@ int bitmap_scnlistprintf(char *buf, unsigned int
buflen,
EXPORT_SYMBOL(bitmap_scnlistprintf);
/**
+ * bitmap_copy_to_pagebuf - convert bitmap to list or hex format ASCII
string
+ * @list: indicates whether the bitmap must be list
+ * @maskp: pointer to bitmap to convert
+ * @buf: page aligned buffer into which string is placed
+ *
+ * Output format is a comma-separated list of decimal numbers and
+ * ranges if list is specified or hex digits grouped into comma-separated
+ * sets of 8 digits/set. Returns the number of characters written to buf.
+ */
+int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char
*buf)
+{
+ ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
+ int n = 0;
+
+ if (len > 1) {
+ n = list ?
+ bitmap_scnlistprintf(buf, len, maskp, nr_cpumask_bits) :
+ bitmap_scnprintf(buf, len, maskp, nr_cpumask_bits);
+ buf[n++] = '\n';
+ buf[n] = '\0';
+ }
+ return n;
+}
+EXPORT_SYMBOL(bitmap_copy_to_pagebuf);
+
+/**
* __bitmap_parselist - convert list format ASCII string to bitmap
* @buf: read nul-terminated user string from this buffer
* @buflen: buffer size in bytes. If string is smaller than this
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-04 10:43 ` Sudeep Holla
@ 2014-09-04 11:25 ` Peter Zijlstra
2014-09-04 12:27 ` Sudeep Holla
0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2014-09-04 11:25 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Heiko Carstens, Lorenzo Pieralisi, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, x86, linux-acpi, linux-pci
On Thu, Sep 04, 2014 at 11:43:18AM +0100, Sudeep Holla wrote:
> >You could make it bitmap_copy_to_buf() I suppose, and then have a
> >cpumask inline wrapper, to be fully consistent with the rest of the
> >implementations.
> >
>
> I liked the second approach, how about patch below ?
> If will munge it into original patch.
Looks good, one more nit if you will permit :-)
> +int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char *buf)
Maybe s/copy/print/ or something? Its not really a 'copy' as such since
it very much transforms the bitmap.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function
2014-09-04 11:25 ` Peter Zijlstra
@ 2014-09-04 12:27 ` Sudeep Holla
0 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2014-09-04 12:27 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sudeep Holla, LKML, Heiko Carstens, Lorenzo Pieralisi,
Greg Kroah-Hartman, Rafael J. Wysocki, Bjorn Helgaas, x86,
linux-acpi, linux-pci
On 04/09/14 12:25, Peter Zijlstra wrote:
> On Thu, Sep 04, 2014 at 11:43:18AM +0100, Sudeep Holla wrote:
>>> You could make it bitmap_copy_to_buf() I suppose, and then have a
>>> cpumask inline wrapper, to be fully consistent with the rest of the
>>> implementations.
>>>
>>
>> I liked the second approach, how about patch below ?
>> If will munge it into original patch.
>
> Looks good, one more nit if you will permit :-)
>
Ofcourse :)
>> +int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char *buf)
>
> Maybe s/copy/print/ or something? Its not really a 'copy' as such since
> it very much transforms the bitmap.
>
Makes sense, will update accordingly.
Regards,
Sudeep
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 01/11 UPDATE] cpumask: factor out show_cpumap into separate helper function
2014-09-03 17:00 ` [PATCH v4 01/11] cpumask: factor out show_cpumap into separate helper function Sudeep Holla
` (2 preceding siblings ...)
2014-09-04 6:20 ` Peter Zijlstra
@ 2014-09-04 15:46 ` Sudeep Holla
2014-09-19 22:23 ` Stephen Boyd
` (2 more replies)
3 siblings, 3 replies; 14+ messages in thread
From: Sudeep Holla @ 2014-09-04 15:46 UTC (permalink / raw)
To: LKML
Cc: Sudeep Holla, Lorenzo Pieralisi, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, Peter Zijlstra, x86,
linux-acpi, linux-pci
From: Sudeep Holla <sudeep.holla@arm.com>
Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
terminated buffer with newline.
This patch creates a new helper function cpumap_print_to_pagebuf in
cpumask.h using newly added bitmap_print_to_pagebuf and consolidates
most of those sysfs functions using the new helper function.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: x86@kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-pci@vger.kernel.org
---
arch/x86/kernel/cpu/perf_event_amd_iommu.c | 5 +---
arch/x86/kernel/cpu/perf_event_amd_uncore.c | 6 +----
arch/x86/kernel/cpu/perf_event_intel_rapl.c | 6 +----
arch/x86/kernel/cpu/perf_event_intel_uncore.c | 6 +----
drivers/acpi/acpi_pad.c | 8 +++---
drivers/base/cpu.c | 5 +---
drivers/base/node.c | 14 +++-------
drivers/base/topology.c | 22 ++-------------
drivers/pci/pci-sysfs.c | 39 +++++++--------------------
include/linux/bitmap.h | 3 +++
include/linux/cpumask.h | 17 ++++++++++++
lib/bitmap.c | 29 ++++++++++++++++++++
12 files changed, 73 insertions(+), 87 deletions(-)
Hi,
I am sending just update(instead of new version) to this patch as it's
independent of the series and can be taken out of it. It is just to
avoid another churn of the series without much update.
Regards,
Sudeep
diff --git a/arch/x86/kernel/cpu/perf_event_amd_iommu.c b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
index 639d1289b1ba..97242a9242bd 100644
--- a/arch/x86/kernel/cpu/perf_event_amd_iommu.c
+++ b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
@@ -130,10 +130,7 @@ static ssize_t _iommu_cpumask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &iommu_cpumask);
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(true, buf, &iommu_cpumask);
}
static DEVICE_ATTR(cpumask, S_IRUGO, _iommu_cpumask_show, NULL);
diff --git a/arch/x86/kernel/cpu/perf_event_amd_uncore.c b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
index 30790d798e6b..cc6cedb8f25d 100644
--- a/arch/x86/kernel/cpu/perf_event_amd_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
@@ -219,7 +219,6 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- int n;
cpumask_t *active_mask;
struct pmu *pmu = dev_get_drvdata(dev);
@@ -230,10 +229,7 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
else
return 0;
- n = cpulist_scnprintf(buf, PAGE_SIZE - 2, active_mask);
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(true, buf, active_mask);
}
static DEVICE_ATTR(cpumask, S_IRUGO, amd_uncore_attr_show_cpumask, NULL);
diff --git a/arch/x86/kernel/cpu/perf_event_intel_rapl.c b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
index 619f7699487a..3256cb87834e 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_rapl.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
@@ -365,11 +365,7 @@ static void rapl_pmu_event_read(struct perf_event *event)
static ssize_t rapl_get_attr_cpumask(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &rapl_cpu_mask);
-
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(true, buf, &rapl_cpu_mask);
}
static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 0939f86f543d..3c8ddf0b89ad 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -3617,11 +3617,7 @@ static int uncore_pmu_event_init(struct perf_event *event)
static ssize_t uncore_get_attr_cpumask(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
-
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(true, buf, &uncore_cpu_mask);
}
static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index f148a0580e04..c7b105c0e1d3 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -350,12 +350,10 @@ static ssize_t acpi_pad_idlecpus_store(struct device *dev,
static ssize_t acpi_pad_idlecpus_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n = 0;
- n = cpumask_scnprintf(buf, PAGE_SIZE-2, to_cpumask(pad_busy_cpus_bits));
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(false, buf,
+ to_cpumask(pad_busy_cpus_bits));
}
+
static DEVICE_ATTR(idlecpus, S_IRUGO|S_IWUSR,
acpi_pad_idlecpus_show,
acpi_pad_idlecpus_store);
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 006b1bc5297d..4d8a56406fbb 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -207,11 +207,8 @@ static ssize_t show_cpus_attr(struct device *dev,
char *buf)
{
struct cpu_attr *ca = container_of(attr, struct cpu_attr, attr);
- int n = cpulist_scnprintf(buf, PAGE_SIZE-2, *(ca->map));
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
+ return cpumap_print_to_pagebuf(true, buf, *ca->map);
}
#define _CPU_ATTR(name, map) \
diff --git a/drivers/base/node.c b/drivers/base/node.c
index c6d3ae05f1ca..e68c134551ea 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -25,32 +25,26 @@ static struct bus_type node_subsys = {
};
-static ssize_t node_read_cpumap(struct device *dev, int type, char *buf)
+static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
{
struct node *node_dev = to_node(dev);
const struct cpumask *mask = cpumask_of_node(node_dev->dev.id);
- int len;
/* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
- len = type?
- cpulist_scnprintf(buf, PAGE_SIZE-2, mask) :
- cpumask_scnprintf(buf, PAGE_SIZE-2, mask);
- buf[len++] = '\n';
- buf[len] = '\0';
- return len;
+ return cpumap_print_to_pagebuf(list, buf, mask);
}
static inline ssize_t node_read_cpumask(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return node_read_cpumap(dev, 0, buf);
+ return node_read_cpumap(dev, false, buf);
}
static inline ssize_t node_read_cpulist(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return node_read_cpumap(dev, 1, buf);
+ return node_read_cpumap(dev, true, buf);
}
static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask, NULL);
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index be7c1fb7c0c9..f7c353843ddf 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -42,29 +42,11 @@ static ssize_t show_##name(struct device *dev, \
return sprintf(buf, "%d\n", topology_##name(dev->id)); \
}
-#if defined(topology_thread_cpumask) || defined(topology_core_cpumask) || \
- defined(topology_book_cpumask)
-static ssize_t show_cpumap(int type, const struct cpumask *mask, char *buf)
-{
- ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
- int n = 0;
-
- if (len > 1) {
- n = type?
- cpulist_scnprintf(buf, len-2, mask) :
- cpumask_scnprintf(buf, len-2, mask);
- buf[n++] = '\n';
- buf[n] = '\0';
- }
- return n;
-}
-#endif
-
#define define_siblings_show_map(name) \
static ssize_t show_##name(struct device *dev, \
struct device_attribute *attr, char *buf) \
{ \
- return show_cpumap(0, topology_##name(dev->id), buf); \
+ return cpumap_print_to_pagebuf(false, buf, topology_##name(dev->id));\
}
#define define_siblings_show_list(name) \
@@ -72,7 +54,7 @@ static ssize_t show_##name##_list(struct device *dev, \
struct device_attribute *attr, \
char *buf) \
{ \
- return show_cpumap(1, topology_##name(dev->id), buf); \
+ return cpumap_print_to_pagebuf(true, buf, topology_##name(dev->id));\
}
#define define_siblings_show_func(name) \
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a901ecf7..b8c0ffb36ae7 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -77,11 +77,10 @@ static ssize_t broken_parity_status_store(struct device *dev,
}
static DEVICE_ATTR_RW(broken_parity_status);
-static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
+static ssize_t pci_dev_show_local_cpu(struct device *dev, bool list,
struct device_attribute *attr, char *buf)
{
const struct cpumask *mask;
- int len;
#ifdef CONFIG_NUMA
mask = (dev_to_node(dev) == -1) ? cpu_online_mask :
@@ -89,59 +88,41 @@ static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
#else
mask = cpumask_of_pcibus(to_pci_dev(dev)->bus);
#endif
- len = type ?
- cpumask_scnprintf(buf, PAGE_SIZE-2, mask) :
- cpulist_scnprintf(buf, PAGE_SIZE-2, mask);
-
- buf[len++] = '\n';
- buf[len] = '\0';
- return len;
+ return cpumap_print_to_pagebuf(list, buf, mask);
}
static ssize_t local_cpus_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return pci_dev_show_local_cpu(dev, 1, attr, buf);
+ return pci_dev_show_local_cpu(dev, false, attr, buf);
}
static DEVICE_ATTR_RO(local_cpus);
static ssize_t local_cpulist_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return pci_dev_show_local_cpu(dev, 0, attr, buf);
+ return pci_dev_show_local_cpu(dev, true, attr, buf);
}
static DEVICE_ATTR_RO(local_cpulist);
/*
* PCI Bus Class Devices
*/
-static ssize_t pci_bus_show_cpuaffinity(struct device *dev, int type,
- struct device_attribute *attr,
- char *buf)
-{
- int ret;
- const struct cpumask *cpumask;
-
- cpumask = cpumask_of_pcibus(to_pci_bus(dev));
- ret = type ?
- cpulist_scnprintf(buf, PAGE_SIZE-2, cpumask) :
- cpumask_scnprintf(buf, PAGE_SIZE-2, cpumask);
- buf[ret++] = '\n';
- buf[ret] = '\0';
- return ret;
-}
-
static ssize_t cpuaffinity_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return pci_bus_show_cpuaffinity(dev, 0, attr, buf);
+ const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
+
+ return cpumap_print_to_pagebuf(false, buf, cpumask);
}
static DEVICE_ATTR_RO(cpuaffinity);
static ssize_t cpulistaffinity_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return pci_bus_show_cpuaffinity(dev, 1, attr, buf);
+ const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
+
+ return cpumap_print_to_pagebuf(true, buf, cpumask);
}
static DEVICE_ATTR_RO(cpulistaffinity);
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e1c8d080c427..9d5c3224a1e2 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -60,6 +60,7 @@
* bitmap_find_free_region(bitmap, bits, order) Find and allocate bit region
* bitmap_release_region(bitmap, pos, order) Free specified bit region
* bitmap_allocate_region(bitmap, pos, order) Allocate specified bit region
+ * bitmap_print_to_pagebuf(list, buf, mask, nbits) Print bitmap src as list/hex
*/
/*
@@ -145,6 +146,8 @@ extern void bitmap_release_region(unsigned long *bitmap, unsigned int pos, int o
extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order);
extern void bitmap_copy_le(void *dst, const unsigned long *src, int nbits);
extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int bits);
+extern int bitmap_print_to_pagebuf(bool list, char *buf,
+ const unsigned long *maskp, int nmaskbits);
#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
#define BITMAP_LAST_WORD_MASK(nbits) \
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 2997af6d2ccd..d8a6f8a1d308 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -792,6 +792,23 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
}
#endif /* NR_CPUS > BITS_PER_LONG */
+/**
+ * cpumap_print_to_pagebuf - copies the cpumask into the buffer either
+ * as comma-separated list of cpus or hex values of cpumask
+ * @list: indicates whether the cpumap must be list
+ * @mask: the cpumask to copy
+ * @buf: the buffer to copy into
+ *
+ * Returns the length of the (null-terminated) @buf string, zero if
+ * nothing is copied.
+ */
+static inline ssize_t
+cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
+{
+ return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
+ nr_cpumask_bits);
+}
+
/*
*
* From here down, all obsolete. Use cpumask_ variants!
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 1e031f2c9aba..0a9f39388c63 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -12,6 +12,8 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
#include <linux/bug.h>
+
+#include <asm/page.h>
#include <asm/uaccess.h>
/*
@@ -580,6 +582,33 @@ int bitmap_scnlistprintf(char *buf, unsigned int buflen,
EXPORT_SYMBOL(bitmap_scnlistprintf);
/**
+ * bitmap_print_to_pagebuf - convert bitmap to list or hex format ASCII string
+ * @list: indicates whether the bitmap must be list
+ * @buf: page aligned buffer into which string is placed
+ * @maskp: pointer to bitmap to convert
+ * @nmaskbits: size of bitmap, in bits
+ *
+ * Output format is a comma-separated list of decimal numbers and
+ * ranges if list is specified or hex digits grouped into comma-separated
+ * sets of 8 digits/set. Returns the number of characters written to buf.
+ */
+int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
+ int nmaskbits)
+{
+ ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
+ int n = 0;
+
+ if (len > 1) {
+ n = list ? bitmap_scnlistprintf(buf, len, maskp, nmaskbits) :
+ bitmap_scnprintf(buf, len, maskp, nmaskbits);
+ buf[n++] = '\n';
+ buf[n] = '\0';
+ }
+ return n;
+}
+EXPORT_SYMBOL(bitmap_print_to_pagebuf);
+
+/**
* __bitmap_parselist - convert list format ASCII string to bitmap
* @buf: read nul-terminated user string from this buffer
* @buflen: buffer size in bytes. If string is smaller than this
--
1.8.3.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11 UPDATE] cpumask: factor out show_cpumap into separate helper function
2014-09-04 15:46 ` [PATCH v4 01/11 UPDATE] " Sudeep Holla
@ 2014-09-19 22:23 ` Stephen Boyd
2014-09-24 8:51 ` Sudeep Holla
2014-09-24 10:20 ` Peter Zijlstra
2 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2014-09-19 22:23 UTC (permalink / raw)
To: Sudeep Holla, LKML
Cc: Lorenzo Pieralisi, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, Peter Zijlstra, x86, linux-acpi, linux-pci
On 09/04/14 08:46, Sudeep Holla wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
> terminated buffer with newline.
>
> This patch creates a new helper function cpumap_print_to_pagebuf in
> cpumask.h using newly added bitmap_print_to_pagebuf and consolidates
> most of those sysfs functions using the new helper function.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: x86@kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
>
Tested-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11 UPDATE] cpumask: factor out show_cpumap into separate helper function
2014-09-04 15:46 ` [PATCH v4 01/11 UPDATE] " Sudeep Holla
2014-09-19 22:23 ` Stephen Boyd
@ 2014-09-24 8:51 ` Sudeep Holla
2014-09-24 10:20 ` Peter Zijlstra
2 siblings, 0 replies; 14+ messages in thread
From: Sudeep Holla @ 2014-09-24 8:51 UTC (permalink / raw)
To: LKML, Peter Zijlstra
Cc: Sudeep Holla, Lorenzo Pieralisi, Greg Kroah-Hartman,
Rafael J. Wysocki, Bjorn Helgaas, x86, linux-acpi, linux-pci
Hi Peter,
On 04/09/14 16:46, Sudeep Holla wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
> terminated buffer with newline.
>
> This patch creates a new helper function cpumap_print_to_pagebuf in
> cpumask.h using newly added bitmap_print_to_pagebuf and consolidates
> most of those sysfs functions using the new helper function.
>
You had mentioned that you were fine with the changes, can I get
Acked-by then ?
Regards,
Sudeep
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: x86@kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-pci@vger.kernel.org
> ---
> arch/x86/kernel/cpu/perf_event_amd_iommu.c | 5 +---
> arch/x86/kernel/cpu/perf_event_amd_uncore.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_rapl.c | 6 +----
> arch/x86/kernel/cpu/perf_event_intel_uncore.c | 6 +----
> drivers/acpi/acpi_pad.c | 8 +++---
> drivers/base/cpu.c | 5 +---
> drivers/base/node.c | 14 +++-------
> drivers/base/topology.c | 22 ++-------------
> drivers/pci/pci-sysfs.c | 39 +++++++--------------------
> include/linux/bitmap.h | 3 +++
> include/linux/cpumask.h | 17 ++++++++++++
> lib/bitmap.c | 29 ++++++++++++++++++++
> 12 files changed, 73 insertions(+), 87 deletions(-)
>
> Hi,
>
> I am sending just update(instead of new version) to this patch as it's
> independent of the series and can be taken out of it. It is just to
> avoid another churn of the series without much update.
>
> Regards,
> Sudeep
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_iommu.c b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> index 639d1289b1ba..97242a9242bd 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_iommu.c
> @@ -130,10 +130,7 @@ static ssize_t _iommu_cpumask_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &iommu_cpumask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(true, buf, &iommu_cpumask);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, _iommu_cpumask_show, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_amd_uncore.c b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> index 30790d798e6b..cc6cedb8f25d 100644
> --- a/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_amd_uncore.c
> @@ -219,7 +219,6 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - int n;
> cpumask_t *active_mask;
> struct pmu *pmu = dev_get_drvdata(dev);
>
> @@ -230,10 +229,7 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
> else
> return 0;
>
> - n = cpulist_scnprintf(buf, PAGE_SIZE - 2, active_mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(true, buf, active_mask);
> }
> static DEVICE_ATTR(cpumask, S_IRUGO, amd_uncore_attr_show_cpumask, NULL);
>
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_rapl.c b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> index 619f7699487a..3256cb87834e 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_rapl.c
> @@ -365,11 +365,7 @@ static void rapl_pmu_event_read(struct perf_event *event)
> static ssize_t rapl_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &rapl_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(true, buf, &rapl_cpu_mask);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0939f86f543d..3c8ddf0b89ad 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -3617,11 +3617,7 @@ static int uncore_pmu_event_init(struct perf_event *event)
> static ssize_t uncore_get_attr_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
> -
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(true, buf, &uncore_cpu_mask);
> }
>
> static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
> diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> index f148a0580e04..c7b105c0e1d3 100644
> --- a/drivers/acpi/acpi_pad.c
> +++ b/drivers/acpi/acpi_pad.c
> @@ -350,12 +350,10 @@ static ssize_t acpi_pad_idlecpus_store(struct device *dev,
> static ssize_t acpi_pad_idlecpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - int n = 0;
> - n = cpumask_scnprintf(buf, PAGE_SIZE-2, to_cpumask(pad_busy_cpus_bits));
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(false, buf,
> + to_cpumask(pad_busy_cpus_bits));
> }
> +
> static DEVICE_ATTR(idlecpus, S_IRUGO|S_IWUSR,
> acpi_pad_idlecpus_show,
> acpi_pad_idlecpus_store);
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 006b1bc5297d..4d8a56406fbb 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -207,11 +207,8 @@ static ssize_t show_cpus_attr(struct device *dev,
> char *buf)
> {
> struct cpu_attr *ca = container_of(attr, struct cpu_attr, attr);
> - int n = cpulist_scnprintf(buf, PAGE_SIZE-2, *(ca->map));
>
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - return n;
> + return cpumap_print_to_pagebuf(true, buf, *ca->map);
> }
>
> #define _CPU_ATTR(name, map) \
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index c6d3ae05f1ca..e68c134551ea 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -25,32 +25,26 @@ static struct bus_type node_subsys = {
> };
>
>
> -static ssize_t node_read_cpumap(struct device *dev, int type, char *buf)
> +static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
> {
> struct node *node_dev = to_node(dev);
> const struct cpumask *mask = cpumask_of_node(node_dev->dev.id);
> - int len;
>
> /* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
> BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
>
> - len = type?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask);
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_print_to_pagebuf(list, buf, mask);
> }
>
> static inline ssize_t node_read_cpumask(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 0, buf);
> + return node_read_cpumap(dev, false, buf);
> }
> static inline ssize_t node_read_cpulist(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return node_read_cpumap(dev, 1, buf);
> + return node_read_cpumap(dev, true, buf);
> }
>
> static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask, NULL);
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index be7c1fb7c0c9..f7c353843ddf 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -42,29 +42,11 @@ static ssize_t show_##name(struct device *dev, \
> return sprintf(buf, "%d\n", topology_##name(dev->id)); \
> }
>
> -#if defined(topology_thread_cpumask) || defined(topology_core_cpumask) || \
> - defined(topology_book_cpumask)
> -static ssize_t show_cpumap(int type, const struct cpumask *mask, char *buf)
> -{
> - ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf;
> - int n = 0;
> -
> - if (len > 1) {
> - n = type?
> - cpulist_scnprintf(buf, len-2, mask) :
> - cpumask_scnprintf(buf, len-2, mask);
> - buf[n++] = '\n';
> - buf[n] = '\0';
> - }
> - return n;
> -}
> -#endif
> -
> #define define_siblings_show_map(name) \
> static ssize_t show_##name(struct device *dev, \
> struct device_attribute *attr, char *buf) \
> { \
> - return show_cpumap(0, topology_##name(dev->id), buf); \
> + return cpumap_print_to_pagebuf(false, buf, topology_##name(dev->id));\
> }
>
> #define define_siblings_show_list(name) \
> @@ -72,7 +54,7 @@ static ssize_t show_##name##_list(struct device *dev, \
> struct device_attribute *attr, \
> char *buf) \
> { \
> - return show_cpumap(1, topology_##name(dev->id), buf); \
> + return cpumap_print_to_pagebuf(true, buf, topology_##name(dev->id));\
> }
>
> #define define_siblings_show_func(name) \
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9ff0a901ecf7..b8c0ffb36ae7 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -77,11 +77,10 @@ static ssize_t broken_parity_status_store(struct device *dev,
> }
> static DEVICE_ATTR_RW(broken_parity_status);
>
> -static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> +static ssize_t pci_dev_show_local_cpu(struct device *dev, bool list,
> struct device_attribute *attr, char *buf)
> {
> const struct cpumask *mask;
> - int len;
>
> #ifdef CONFIG_NUMA
> mask = (dev_to_node(dev) == -1) ? cpu_online_mask :
> @@ -89,59 +88,41 @@ static ssize_t pci_dev_show_local_cpu(struct device *dev, int type,
> #else
> mask = cpumask_of_pcibus(to_pci_dev(dev)->bus);
> #endif
> - len = type ?
> - cpumask_scnprintf(buf, PAGE_SIZE-2, mask) :
> - cpulist_scnprintf(buf, PAGE_SIZE-2, mask);
> -
> - buf[len++] = '\n';
> - buf[len] = '\0';
> - return len;
> + return cpumap_print_to_pagebuf(list, buf, mask);
> }
>
> static ssize_t local_cpus_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 1, attr, buf);
> + return pci_dev_show_local_cpu(dev, false, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpus);
>
> static ssize_t local_cpulist_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_dev_show_local_cpu(dev, 0, attr, buf);
> + return pci_dev_show_local_cpu(dev, true, attr, buf);
> }
> static DEVICE_ATTR_RO(local_cpulist);
>
> /*
> * PCI Bus Class Devices
> */
> -static ssize_t pci_bus_show_cpuaffinity(struct device *dev, int type,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - int ret;
> - const struct cpumask *cpumask;
> -
> - cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> - ret = type ?
> - cpulist_scnprintf(buf, PAGE_SIZE-2, cpumask) :
> - cpumask_scnprintf(buf, PAGE_SIZE-2, cpumask);
> - buf[ret++] = '\n';
> - buf[ret] = '\0';
> - return ret;
> -}
> -
> static ssize_t cpuaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 0, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_print_to_pagebuf(false, buf, cpumask);
> }
> static DEVICE_ATTR_RO(cpuaffinity);
>
> static ssize_t cpulistaffinity_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - return pci_bus_show_cpuaffinity(dev, 1, attr, buf);
> + const struct cpumask *cpumask = cpumask_of_pcibus(to_pci_bus(dev));
> +
> + return cpumap_print_to_pagebuf(true, buf, cpumask);
> }
> static DEVICE_ATTR_RO(cpulistaffinity);
>
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index e1c8d080c427..9d5c3224a1e2 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -60,6 +60,7 @@
> * bitmap_find_free_region(bitmap, bits, order) Find and allocate bit region
> * bitmap_release_region(bitmap, pos, order) Free specified bit region
> * bitmap_allocate_region(bitmap, pos, order) Allocate specified bit region
> + * bitmap_print_to_pagebuf(list, buf, mask, nbits) Print bitmap src as list/hex
> */
>
> /*
> @@ -145,6 +146,8 @@ extern void bitmap_release_region(unsigned long *bitmap, unsigned int pos, int o
> extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order);
> extern void bitmap_copy_le(void *dst, const unsigned long *src, int nbits);
> extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int bits);
> +extern int bitmap_print_to_pagebuf(bool list, char *buf,
> + const unsigned long *maskp, int nmaskbits);
>
> #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
> #define BITMAP_LAST_WORD_MASK(nbits) \
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 2997af6d2ccd..d8a6f8a1d308 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -792,6 +792,23 @@ static inline const struct cpumask *get_cpu_mask(unsigned int cpu)
> }
> #endif /* NR_CPUS > BITS_PER_LONG */
>
> +/**
> + * cpumap_print_to_pagebuf - copies the cpumask into the buffer either
> + * as comma-separated list of cpus or hex values of cpumask
> + * @list: indicates whether the cpumap must be list
> + * @mask: the cpumask to copy
> + * @buf: the buffer to copy into
> + *
> + * Returns the length of the (null-terminated) @buf string, zero if
> + * nothing is copied.
> + */
> +static inline ssize_t
> +cpumap_print_to_pagebuf(bool list, char *buf, const struct cpumask *mask)
> +{
> + return bitmap_print_to_pagebuf(list, buf, cpumask_bits(mask),
> + nr_cpumask_bits);
> +}
> +
> /*
> *
> * From here down, all obsolete. Use cpumask_ variants!
> diff --git a/lib/bitmap.c b/lib/bitmap.c
> index 1e031f2c9aba..0a9f39388c63 100644
> --- a/lib/bitmap.c
> +++ b/lib/bitmap.c
> @@ -12,6 +12,8 @@
> #include <linux/bitmap.h>
> #include <linux/bitops.h>
> #include <linux/bug.h>
> +
> +#include <asm/page.h>
> #include <asm/uaccess.h>
>
> /*
> @@ -580,6 +582,33 @@ int bitmap_scnlistprintf(char *buf, unsigned int buflen,
> EXPORT_SYMBOL(bitmap_scnlistprintf);
>
> /**
> + * bitmap_print_to_pagebuf - convert bitmap to list or hex format ASCII string
> + * @list: indicates whether the bitmap must be list
> + * @buf: page aligned buffer into which string is placed
> + * @maskp: pointer to bitmap to convert
> + * @nmaskbits: size of bitmap, in bits
> + *
> + * Output format is a comma-separated list of decimal numbers and
> + * ranges if list is specified or hex digits grouped into comma-separated
> + * sets of 8 digits/set. Returns the number of characters written to buf.
> + */
> +int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp,
> + int nmaskbits)
> +{
> + ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
> + int n = 0;
> +
> + if (len > 1) {
> + n = list ? bitmap_scnlistprintf(buf, len, maskp, nmaskbits) :
> + bitmap_scnprintf(buf, len, maskp, nmaskbits);
> + buf[n++] = '\n';
> + buf[n] = '\0';
> + }
> + return n;
> +}
> +EXPORT_SYMBOL(bitmap_print_to_pagebuf);
> +
> +/**
> * __bitmap_parselist - convert list format ASCII string to bitmap
> * @buf: read nul-terminated user string from this buffer
> * @buflen: buffer size in bytes. If string is smaller than this
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 01/11 UPDATE] cpumask: factor out show_cpumap into separate helper function
2014-09-04 15:46 ` [PATCH v4 01/11 UPDATE] " Sudeep Holla
2014-09-19 22:23 ` Stephen Boyd
2014-09-24 8:51 ` Sudeep Holla
@ 2014-09-24 10:20 ` Peter Zijlstra
2 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2014-09-24 10:20 UTC (permalink / raw)
To: Sudeep Holla
Cc: LKML, Lorenzo Pieralisi, Greg Kroah-Hartman, Rafael J. Wysocki,
Bjorn Helgaas, x86, linux-acpi, linux-pci
On Thu, Sep 04, 2014 at 04:46:32PM +0100, Sudeep Holla wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> Many sysfs *_show function use cpu{list,mask}_scnprintf to copy cpumap
> to the buffer aligned to PAGE_SIZE, append '\n' and '\0' to return null
> terminated buffer with newline.
>
> This patch creates a new helper function cpumap_print_to_pagebuf in
> cpumask.h using newly added bitmap_print_to_pagebuf and consolidates
> most of those sysfs functions using the new helper function.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Acked-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 14+ messages in thread