* [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at
@ 2020-09-16 20:40 Joe Perches
2020-09-16 20:40 ` [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output Joe Perches
` (7 more replies)
0 siblings, 8 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, linux-kernel, linux-pm, linux-mm
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-doc
Output defects can exist in sysfs content using sprintf and snprintf.
sprintf does not know the PAGE_SIZE maximum of the temporary buffer
used for outputting sysfs content and it's possible to overrun the
PAGE_SIZE buffer length.
Add a generic sysfs_emit function that knows that the size of the
temporary buffer and ensures that no overrun is done.
Add a generic sysfs_emit_at function that can be used in multiple
call situations that also ensures that no overrun is done.
V2: Add drivers core conversions
V3: Add a few more neatenings and conversions to drivers core
Add mm hugetlb_report_node_meminfo conversion
Joe Perches (8):
sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions
drivers core: Remove strcat uses around sysfs_emit and neaten
drivers core: Reindent a couple uses around sysfs_emit
drivers core: Miscellaneous changes for sysfs_emit
mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show
drivers core: node: Use a more typical macro definition style for ACCESS_ATTR
Documentation/filesystems/sysfs.rst | 8 +-
drivers/base/arch_topology.c | 2 +-
drivers/base/bus.c | 2 +-
drivers/base/cacheinfo.c | 49 ++--
drivers/base/class.c | 2 +-
drivers/base/core.c | 59 +++--
drivers/base/cpu.c | 84 ++++---
drivers/base/dd.c | 3 +-
drivers/base/devcoredump.c | 2 +-
drivers/base/firmware_loader/fallback.c | 4 +-
drivers/base/memory.c | 62 ++---
drivers/base/node.c | 306 ++++++++++++------------
drivers/base/platform.c | 17 +-
drivers/base/power/sysfs.c | 160 ++++++++-----
drivers/base/power/wakeup_stats.c | 17 +-
drivers/base/soc.c | 64 +++--
drivers/base/topology.c | 10 +-
fs/sysfs/file.c | 55 +++++
include/linux/hugetlb.h | 4 +-
include/linux/sysfs.h | 15 ++
mm/hugetlb.c | 18 +-
21 files changed, 532 insertions(+), 411 deletions(-)
--
2.26.0
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-30 11:57 ` Greg Kroah-Hartman
2020-09-16 20:40 ` [PATCH V3 2/8] drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions Joe Perches
` (6 subsequent siblings)
7 siblings, 1 reply; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel,
Jonathan Corbet, linux-doc
Output defects can exist in sysfs content using sprintf and snprintf.
sprintf does not know the PAGE_SIZE maximum of the temporary buffer
used for outputting sysfs content and it's possible to overrun the
PAGE_SIZE buffer length.
Add a generic sysfs_emit function that knows that the size of the
temporary buffer and ensures that no overrun is done.
Add a generic sysfs_emit_at function that can be used in multiple
call situations that also ensures that no overrun is done.
Validate the output buffer argument to be page aligned.
Validate the offset len argument to be within the PAGE_SIZE buf.
Signed-off-by: Joe Perches <joe@perches.com>
---
Documentation/filesystems/sysfs.rst | 8 ++---
fs/sysfs/file.c | 55 +++++++++++++++++++++++++++++
include/linux/sysfs.h | 15 ++++++++
3 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst
index 5a3209a4cebf..004d490179f3 100644
--- a/Documentation/filesystems/sysfs.rst
+++ b/Documentation/filesystems/sysfs.rst
@@ -241,12 +241,10 @@ Other notes:
is 4096.
- show() methods should return the number of bytes printed into the
- buffer. This is the return value of scnprintf().
+ buffer.
-- show() must not use snprintf() when formatting the value to be
- returned to user space. If you can guarantee that an overflow
- will never happen you can use sprintf() otherwise you must use
- scnprintf().
+- show() should only use sysfs_emit() or sysfs_emit_at() when formatting
+ the value to be returned to user space.
- store() should return the number of bytes used from the buffer. If the
entire buffer has been used, just return the count argument.
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index eb6897ab78e7..96d0da65e088 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -15,6 +15,7 @@
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/seq_file.h>
+#include <linux/mm.h>
#include "sysfs.h"
@@ -707,3 +708,57 @@ int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)
return 0;
}
EXPORT_SYMBOL_GPL(sysfs_change_owner);
+
+/**
+ * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer.
+ * @buf: start of PAGE_SIZE buffer.
+ * @fmt: format
+ * @...: optional arguments to @format
+ *
+ *
+ * Returns number of characters written to @buf.
+ */
+int sysfs_emit(char *buf, const char *fmt, ...)
+{
+ va_list args;
+ int len;
+
+ if (WARN(!buf || offset_in_page(buf),
+ "invalid sysfs_emit: buf:%p\n", buf))
+ return 0;
+
+ va_start(args, fmt);
+ len = vscnprintf(buf, PAGE_SIZE, fmt, args);
+ va_end(args);
+
+ return len;
+}
+EXPORT_SYMBOL_GPL(sysfs_emit);
+
+/**
+ * sysfs_emit_at - scnprintf equivalent, aware of PAGE_SIZE buffer.
+ * @buf: start of PAGE_SIZE buffer.
+ * @at: offset in @buf to start write in bytes
+ * @at must be >= 0 && < PAGE_SIZE
+ * @fmt: format
+ * @...: optional arguments to @fmt
+ *
+ *
+ * Returns number of characters written starting at &@buf[@at].
+ */
+int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
+{
+ va_list args;
+ int len;
+
+ if (WARN(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE,
+ "invalid sysfs_emit_at: buf:%p at:%d\n", buf, at))
+ return 0;
+
+ va_start(args, fmt);
+ len = vscnprintf(buf + at, PAGE_SIZE - at, fmt, args);
+ va_end(args);
+
+ return len;
+}
+EXPORT_SYMBOL_GPL(sysfs_emit_at);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 34e84122f635..2caa34c1ca1a 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -329,6 +329,10 @@ int sysfs_groups_change_owner(struct kobject *kobj,
int sysfs_group_change_owner(struct kobject *kobj,
const struct attribute_group *groups, kuid_t kuid,
kgid_t kgid);
+__printf(2, 3)
+int sysfs_emit(char *buf, const char *fmt, ...);
+__printf(3, 4)
+int sysfs_emit_at(char *buf, int at, const char *fmt, ...);
#else /* CONFIG_SYSFS */
@@ -576,6 +580,17 @@ static inline int sysfs_group_change_owner(struct kobject *kobj,
return 0;
}
+__printf(2, 3)
+static inline int sysfs_emit(char *buf, const char *fmt, ...)
+{
+ return 0;
+}
+
+__printf(3, 4)
+static inline int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
+{
+ return 0;
+}
#endif /* CONFIG_SYSFS */
static inline int __must_check sysfs_create_file(struct kobject *kobj,
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 2/8] drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
2020-09-16 20:40 ` [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-16 20:40 ` [PATCH V3 3/8] drivers core: Remove strcat uses around sysfs_emit and neaten Joe Perches
` (5 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Sudeep Holla,
Luis Chamberlain, Len Brown, Pavel Machek
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-pm
Convert the various sprintf fmaily calls in sysfs device show functions
to sysfs_emit and sysfs_emit_at for PAGE_SIZE buffer safety.
Done with:
$ spatch -sp-file sysfs_emit_dev.cocci --in-place --max-width=80 .
And cocci script:
$ cat sysfs_emit_dev.cocci
@@
identifier d_show;
identifier dev, attr, buf;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- sprintf(buf,
+ sysfs_emit(buf,
...);
...>
}
@@
identifier d_show;
identifier dev, attr, buf;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- snprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
}
@@
identifier d_show;
identifier dev, attr, buf;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- scnprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
}
@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
return
- strcpy(buf, chr);
+ sysfs_emit(buf, chr);
...>
}
@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- sprintf(buf,
+ sysfs_emit(buf,
...);
...>
return len;
}
@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- snprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
return len;
}
@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
len =
- scnprintf(buf, PAGE_SIZE,
+ sysfs_emit(buf,
...);
...>
return len;
}
@@
identifier d_show;
identifier dev, attr, buf;
identifier len;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
<...
- len += scnprintf(buf + len, PAGE_SIZE - len,
+ len += sysfs_emit_at(buf, len,
...);
...>
return len;
}
@@
identifier d_show;
identifier dev, attr, buf;
expression chr;
@@
ssize_t d_show(struct device *dev, struct device_attribute *attr, char *buf)
{
...
- strcpy(buf, chr);
- return strlen(buf);
+ return sysfs_emit(buf, chr);
}
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/arch_topology.c | 2 +-
drivers/base/cacheinfo.c | 18 ++++-----
drivers/base/core.c | 19 +++++-----
drivers/base/cpu.c | 32 ++++++++--------
drivers/base/dd.c | 2 +-
drivers/base/firmware_loader/fallback.c | 2 +-
drivers/base/memory.c | 24 ++++++------
drivers/base/node.c | 28 +++++++-------
drivers/base/platform.c | 4 +-
drivers/base/power/sysfs.c | 50 ++++++++++++-------------
drivers/base/power/wakeup_stats.c | 12 +++---
drivers/base/soc.c | 10 ++---
12 files changed, 102 insertions(+), 101 deletions(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 75f72d684294..744d4618db33 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -71,7 +71,7 @@ static ssize_t cpu_capacity_show(struct device *dev,
{
struct cpu *cpu = container_of(dev, struct cpu, dev);
- return sprintf(buf, "%lu\n", topology_get_cpu_scale(cpu->dev.id));
+ return sysfs_emit(buf, "%lu\n", topology_get_cpu_scale(cpu->dev.id));
}
static void update_topology_flags_workfn(struct work_struct *work);
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 8d553c92cd32..6a8c2b5881be 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -377,7 +377,7 @@ static ssize_t size_show(struct device *dev,
{
struct cacheinfo *this_leaf = dev_get_drvdata(dev);
- return sprintf(buf, "%uK\n", this_leaf->size >> 10);
+ return sysfs_emit(buf, "%uK\n", this_leaf->size >> 10);
}
static ssize_t shared_cpumap_show_func(struct device *dev, bool list, char *buf)
@@ -407,11 +407,11 @@ static ssize_t type_show(struct device *dev,
switch (this_leaf->type) {
case CACHE_TYPE_DATA:
- return sprintf(buf, "Data\n");
+ return sysfs_emit(buf, "Data\n");
case CACHE_TYPE_INST:
- return sprintf(buf, "Instruction\n");
+ return sysfs_emit(buf, "Instruction\n");
case CACHE_TYPE_UNIFIED:
- return sprintf(buf, "Unified\n");
+ return sysfs_emit(buf, "Unified\n");
default:
return -EINVAL;
}
@@ -425,11 +425,11 @@ static ssize_t allocation_policy_show(struct device *dev,
int n = 0;
if ((ci_attr & CACHE_READ_ALLOCATE) && (ci_attr & CACHE_WRITE_ALLOCATE))
- n = sprintf(buf, "ReadWriteAllocate\n");
+ n = sysfs_emit(buf, "ReadWriteAllocate\n");
else if (ci_attr & CACHE_READ_ALLOCATE)
- n = sprintf(buf, "ReadAllocate\n");
+ n = sysfs_emit(buf, "ReadAllocate\n");
else if (ci_attr & CACHE_WRITE_ALLOCATE)
- n = sprintf(buf, "WriteAllocate\n");
+ n = sysfs_emit(buf, "WriteAllocate\n");
return n;
}
@@ -441,9 +441,9 @@ static ssize_t write_policy_show(struct device *dev,
int n = 0;
if (ci_attr & CACHE_WRITE_THROUGH)
- n = sprintf(buf, "WriteThrough\n");
+ n = sysfs_emit(buf, "WriteThrough\n");
else if (ci_attr & CACHE_WRITE_BACK)
- n = sprintf(buf, "WriteBack\n");
+ n = sysfs_emit(buf, "WriteBack\n");
return n;
}
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8dd753539c06..e17e8e941462 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -259,7 +259,7 @@ static ssize_t status_show(struct device *dev,
default:
status = "unknown"; break;
}
- return sprintf(buf, "%s\n", status);
+ return sysfs_emit(buf, "%s\n", status);
}
static DEVICE_ATTR_RO(status);
@@ -276,7 +276,7 @@ static ssize_t auto_remove_on_show(struct device *dev,
else
str = "never";
- return sprintf(buf, "%s\n", str);
+ return sysfs_emit(buf, "%s\n", str);
}
static DEVICE_ATTR_RO(auto_remove_on);
@@ -285,7 +285,7 @@ static ssize_t runtime_pm_show(struct device *dev,
{
struct device_link *link = to_devlink(dev);
- return sprintf(buf, "%d\n", !!(link->flags & DL_FLAG_PM_RUNTIME));
+ return sysfs_emit(buf, "%d\n", !!(link->flags & DL_FLAG_PM_RUNTIME));
}
static DEVICE_ATTR_RO(runtime_pm);
@@ -294,7 +294,8 @@ static ssize_t sync_state_only_show(struct device *dev,
{
struct device_link *link = to_devlink(dev);
- return sprintf(buf, "%d\n", !!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
+ return sysfs_emit(buf, "%d\n",
+ !!(link->flags & DL_FLAG_SYNC_STATE_ONLY));
}
static DEVICE_ATTR_RO(sync_state_only);
@@ -1059,7 +1060,7 @@ static ssize_t waiting_for_supplier_show(struct device *dev,
&& dev->links.need_for_probe;
mutex_unlock(&wfs_lock);
device_unlock(dev);
- return sprintf(buf, "%u\n", val);
+ return sysfs_emit(buf, "%u\n", val);
}
static DEVICE_ATTR_RO(waiting_for_supplier);
@@ -1709,7 +1710,7 @@ ssize_t device_show_ulong(struct device *dev,
char *buf)
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%lx\n", *(unsigned long *)(ea->var));
+ return sysfs_emit(buf, "%lx\n", *(unsigned long *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_ulong);
@@ -1739,7 +1740,7 @@ ssize_t device_show_int(struct device *dev,
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%d\n", *(int *)(ea->var));
+ return sysfs_emit(buf, "%d\n", *(int *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_int);
@@ -1760,7 +1761,7 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
{
struct dev_ext_attribute *ea = to_ext_attr(attr);
- return snprintf(buf, PAGE_SIZE, "%d\n", *(bool *)(ea->var));
+ return sysfs_emit(buf, "%d\n", *(bool *)(ea->var));
}
EXPORT_SYMBOL_GPL(device_show_bool);
@@ -1992,7 +1993,7 @@ static ssize_t online_show(struct device *dev, struct device_attribute *attr,
device_lock(dev);
val = !dev->offline;
device_unlock(dev);
- return sprintf(buf, "%u\n", val);
+ return sysfs_emit(buf, "%u\n", val);
}
static ssize_t online_store(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index d2136ab9b14a..232f8146a8c4 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -156,7 +156,7 @@ static ssize_t show_crash_notes(struct device *dev, struct device_attribute *att
* operation should be safe. No locking required.
*/
addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpunum));
- rc = sprintf(buf, "%Lx\n", addr);
+ rc = sysfs_emit(buf, "%Lx\n", addr);
return rc;
}
static DEVICE_ATTR(crash_notes, 0400, show_crash_notes, NULL);
@@ -167,7 +167,7 @@ static ssize_t show_crash_notes_size(struct device *dev,
{
ssize_t rc;
- rc = sprintf(buf, "%zu\n", sizeof(note_buf_t));
+ rc = sysfs_emit(buf, "%zu\n", sizeof(note_buf_t));
return rc;
}
static DEVICE_ATTR(crash_notes_size, 0400, show_crash_notes_size, NULL);
@@ -231,7 +231,7 @@ static struct cpu_attr cpu_attrs[] = {
static ssize_t print_cpus_kernel_max(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%d\n", NR_CPUS - 1);
+ return sysfs_emit(buf, "%d\n", NR_CPUS - 1);
}
static DEVICE_ATTR(kernel_max, 0444, print_cpus_kernel_max, NULL);
@@ -279,7 +279,7 @@ static ssize_t print_cpus_isolated(struct device *dev,
cpumask_andnot(isolated, cpu_possible_mask,
housekeeping_cpumask(HK_FLAG_DOMAIN));
- n = sprintf(buf, "%*pbl\n", cpumask_pr_args(isolated));
+ n = sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(isolated));
free_cpumask_var(isolated);
@@ -291,7 +291,7 @@ static DEVICE_ATTR(isolated, 0444, print_cpus_isolated, NULL);
static ssize_t print_cpus_nohz_full(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%*pbl\n", cpumask_pr_args(tick_nohz_full_mask));
+ return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(tick_nohz_full_mask));
}
static DEVICE_ATTR(nohz_full, 0444, print_cpus_nohz_full, NULL);
#endif
@@ -323,8 +323,8 @@ static ssize_t print_cpu_modalias(struct device *dev,
ssize_t n;
u32 i;
- n = sprintf(buf, "cpu:type:" CPU_FEATURE_TYPEFMT ":feature:",
- CPU_FEATURE_TYPEVAL);
+ n = sysfs_emit(buf, "cpu:type:" CPU_FEATURE_TYPEFMT ":feature:",
+ CPU_FEATURE_TYPEVAL);
for (i = 0; i < MAX_CPU_FEATURES; i++)
if (cpu_have_feature(i)) {
@@ -516,56 +516,56 @@ static void __init cpu_dev_register_generic(void)
ssize_t __weak cpu_show_meltdown(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_spectre_v1(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_spectre_v2(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_spec_store_bypass(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_l1tf(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_mds(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_tsx_async_abort(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_itlb_multihit(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
ssize_t __weak cpu_show_srbds(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "Not affected\n");
+ return sysfs_emit(buf, "Not affected\n");
}
static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 6e7319ca5bde..123a1c3e9f18 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -486,7 +486,7 @@ static ssize_t state_synced_show(struct device *dev,
device_lock(dev);
val = dev->state_synced;
device_unlock(dev);
- return sprintf(buf, "%u\n", val);
+ return sysfs_emit(buf, "%u\n", val);
}
static DEVICE_ATTR_RO(state_synced);
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 283ca2de76d4..7e9598a1577a 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -219,7 +219,7 @@ static ssize_t firmware_loading_show(struct device *dev,
loading = fw_sysfs_loading(fw_sysfs->fw_priv);
mutex_unlock(&fw_lock);
- return sprintf(buf, "%d\n", loading);
+ return sysfs_emit(buf, "%d\n", loading);
}
/**
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index b4c297dd0475..f5c0b64df2e5 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -119,7 +119,7 @@ static ssize_t phys_index_show(struct device *dev,
unsigned long phys_index;
phys_index = mem->start_section_nr / sections_per_block;
- return sprintf(buf, "%08lx\n", phys_index);
+ return sysfs_emit(buf, "%08lx\n", phys_index);
}
/*
@@ -129,7 +129,7 @@ static ssize_t phys_index_show(struct device *dev,
static ssize_t removable_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", (int)IS_ENABLED(CONFIG_MEMORY_HOTREMOVE));
+ return sysfs_emit(buf, "%d\n", (int)IS_ENABLED(CONFIG_MEMORY_HOTREMOVE));
}
/*
@@ -147,17 +147,17 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
*/
switch (mem->state) {
case MEM_ONLINE:
- len = sprintf(buf, "online\n");
+ len = sysfs_emit(buf, "online\n");
break;
case MEM_OFFLINE:
- len = sprintf(buf, "offline\n");
+ len = sysfs_emit(buf, "offline\n");
break;
case MEM_GOING_OFFLINE:
- len = sprintf(buf, "going-offline\n");
+ len = sysfs_emit(buf, "going-offline\n");
break;
default:
- len = sprintf(buf, "ERROR-UNKNOWN-%ld\n",
- mem->state);
+ len = sysfs_emit(buf, "ERROR-UNKNOWN-%ld\n",
+ mem->state);
WARN_ON(1);
break;
}
@@ -303,7 +303,7 @@ static ssize_t phys_device_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct memory_block *mem = to_memory_block(dev);
- return sprintf(buf, "%d\n", mem->phys_device);
+ return sysfs_emit(buf, "%d\n", mem->phys_device);
}
#ifdef CONFIG_MEMORY_HOTREMOVE
@@ -341,7 +341,7 @@ static ssize_t valid_zones_show(struct device *dev,
default_zone = test_pages_in_a_zone(start_pfn,
start_pfn + nr_pages);
if (!default_zone)
- return sprintf(buf, "none\n");
+ return sysfs_emit(buf, "none\n");
strcat(buf, default_zone->name);
goto out;
}
@@ -374,7 +374,7 @@ static DEVICE_ATTR_RO(removable);
static ssize_t block_size_bytes_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%lx\n", memory_block_size_bytes());
+ return sysfs_emit(buf, "%lx\n", memory_block_size_bytes());
}
static DEVICE_ATTR_RO(block_size_bytes);
@@ -386,8 +386,8 @@ static DEVICE_ATTR_RO(block_size_bytes);
static ssize_t auto_online_blocks_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%s\n",
- online_type_to_str[memhp_default_online_type]);
+ return sysfs_emit(buf, "%s\n",
+ online_type_to_str[memhp_default_online_type]);
}
static ssize_t auto_online_blocks_store(struct device *dev,
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 9426b0f1f660..f6bde7acd5fe 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -370,7 +370,7 @@ static ssize_t node_read_meminfo(struct device *dev,
si_meminfo_node(&i, nid);
sreclaimable = node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B);
sunreclaimable = node_page_state_pages(pgdat, NR_SLAB_UNRECLAIMABLE_B);
- n = sprintf(buf,
+ n = sysfs_emit(buf,
"Node %d MemTotal: %8lu kB\n"
"Node %d MemFree: %8lu kB\n"
"Node %d MemUsed: %8lu kB\n"
@@ -477,19 +477,19 @@ static DEVICE_ATTR(meminfo, S_IRUGO, node_read_meminfo, NULL);
static ssize_t node_read_numastat(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf,
- "numa_hit %lu\n"
- "numa_miss %lu\n"
- "numa_foreign %lu\n"
- "interleave_hit %lu\n"
- "local_node %lu\n"
- "other_node %lu\n",
- sum_zone_numa_state(dev->id, NUMA_HIT),
- sum_zone_numa_state(dev->id, NUMA_MISS),
- sum_zone_numa_state(dev->id, NUMA_FOREIGN),
- sum_zone_numa_state(dev->id, NUMA_INTERLEAVE_HIT),
- sum_zone_numa_state(dev->id, NUMA_LOCAL),
- sum_zone_numa_state(dev->id, NUMA_OTHER));
+ return sysfs_emit(buf,
+ "numa_hit %lu\n"
+ "numa_miss %lu\n"
+ "numa_foreign %lu\n"
+ "interleave_hit %lu\n"
+ "local_node %lu\n"
+ "other_node %lu\n",
+ sum_zone_numa_state(dev->id, NUMA_HIT),
+ sum_zone_numa_state(dev->id, NUMA_MISS),
+ sum_zone_numa_state(dev->id, NUMA_FOREIGN),
+ sum_zone_numa_state(dev->id, NUMA_INTERLEAVE_HIT),
+ sum_zone_numa_state(dev->id, NUMA_LOCAL),
+ sum_zone_numa_state(dev->id, NUMA_OTHER));
}
static DEVICE_ATTR(numastat, S_IRUGO, node_read_numastat, NULL);
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index d90d92e63903..f0f8cfcc3621 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1084,7 +1084,7 @@ static ssize_t driver_override_show(struct device *dev,
ssize_t len;
device_lock(dev);
- len = sprintf(buf, "%s\n", pdev->driver_override);
+ len = sysfs_emit(buf, "%s\n", pdev->driver_override);
device_unlock(dev);
return len;
}
@@ -1093,7 +1093,7 @@ static DEVICE_ATTR_RW(driver_override);
static ssize_t numa_node_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%d\n", dev_to_node(dev));
+ return sysfs_emit(buf, "%d\n", dev_to_node(dev));
}
static DEVICE_ATTR_RO(numa_node);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index c7b24812523c..4276b792d0aa 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -101,7 +101,7 @@ static const char ctrl_on[] = "on";
static ssize_t control_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%s\n",
+ return sysfs_emit(buf, "%s\n",
dev->power.runtime_auto ? ctrl_auto : ctrl_on);
}
@@ -127,7 +127,7 @@ static ssize_t runtime_active_time_show(struct device *dev,
int ret;
u64 tmp = pm_runtime_active_time(dev);
do_div(tmp, NSEC_PER_MSEC);
- ret = sprintf(buf, "%llu\n", tmp);
+ ret = sysfs_emit(buf, "%llu\n", tmp);
return ret;
}
@@ -139,7 +139,7 @@ static ssize_t runtime_suspended_time_show(struct device *dev,
int ret;
u64 tmp = pm_runtime_suspended_time(dev);
do_div(tmp, NSEC_PER_MSEC);
- ret = sprintf(buf, "%llu\n", tmp);
+ ret = sysfs_emit(buf, "%llu\n", tmp);
return ret;
}
@@ -172,7 +172,7 @@ static ssize_t runtime_status_show(struct device *dev,
return -EIO;
}
}
- return sprintf(buf, p);
+ return sysfs_emit(buf, p);
}
static DEVICE_ATTR_RO(runtime_status);
@@ -182,7 +182,7 @@ static ssize_t autosuspend_delay_ms_show(struct device *dev,
{
if (!dev->power.use_autosuspend)
return -EIO;
- return sprintf(buf, "%d\n", dev->power.autosuspend_delay);
+ return sysfs_emit(buf, "%d\n", dev->power.autosuspend_delay);
}
static ssize_t autosuspend_delay_ms_store(struct device *dev,
@@ -211,11 +211,11 @@ static ssize_t pm_qos_resume_latency_us_show(struct device *dev,
s32 value = dev_pm_qos_requested_resume_latency(dev);
if (value == 0)
- return sprintf(buf, "n/a\n");
+ return sysfs_emit(buf, "n/a\n");
if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
value = 0;
- return sprintf(buf, "%d\n", value);
+ return sysfs_emit(buf, "%d\n", value);
}
static ssize_t pm_qos_resume_latency_us_store(struct device *dev,
@@ -255,11 +255,11 @@ static ssize_t pm_qos_latency_tolerance_us_show(struct device *dev,
s32 value = dev_pm_qos_get_user_latency_tolerance(dev);
if (value < 0)
- return sprintf(buf, "auto\n");
+ return sysfs_emit(buf, "auto\n");
if (value == PM_QOS_LATENCY_ANY)
- return sprintf(buf, "any\n");
+ return sysfs_emit(buf, "any\n");
- return sprintf(buf, "%d\n", value);
+ return sysfs_emit(buf, "%d\n", value);
}
static ssize_t pm_qos_latency_tolerance_us_store(struct device *dev,
@@ -291,8 +291,8 @@ static ssize_t pm_qos_no_power_off_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", !!(dev_pm_qos_requested_flags(dev)
- & PM_QOS_FLAG_NO_POWER_OFF));
+ return sysfs_emit(buf, "%d\n", !!(dev_pm_qos_requested_flags(dev)
+ & PM_QOS_FLAG_NO_POWER_OFF));
}
static ssize_t pm_qos_no_power_off_store(struct device *dev,
@@ -320,9 +320,9 @@ static const char _disabled[] = "disabled";
static ssize_t wakeup_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%s\n", device_can_wakeup(dev)
- ? (device_may_wakeup(dev) ? _enabled : _disabled)
- : "");
+ return sysfs_emit(buf, "%s\n", device_can_wakeup(dev)
+ ? (device_may_wakeup(dev) ? _enabled : _disabled)
+ : "");
}
static ssize_t wakeup_store(struct device *dev, struct device_attribute *attr,
@@ -522,7 +522,7 @@ static inline int dpm_sysfs_wakeup_change_owner(struct device *dev, kuid_t kuid,
static ssize_t runtime_usage_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return sprintf(buf, "%d\n", atomic_read(&dev->power.usage_count));
+ return sysfs_emit(buf, "%d\n", atomic_read(&dev->power.usage_count));
}
static DEVICE_ATTR_RO(runtime_usage);
@@ -530,8 +530,8 @@ static ssize_t runtime_active_kids_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", dev->power.ignore_children ?
- 0 : atomic_read(&dev->power.child_count));
+ return sysfs_emit(buf, "%d\n", dev->power.ignore_children ?
+ 0 : atomic_read(&dev->power.child_count));
}
static DEVICE_ATTR_RO(runtime_active_kids);
@@ -539,12 +539,12 @@ static ssize_t runtime_enabled_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
if (dev->power.disable_depth && (dev->power.runtime_auto == false))
- return sprintf(buf, "disabled & forbidden\n");
+ return sysfs_emit(buf, "disabled & forbidden\n");
if (dev->power.disable_depth)
- return sprintf(buf, "disabled\n");
+ return sysfs_emit(buf, "disabled\n");
if (dev->power.runtime_auto == false)
- return sprintf(buf, "forbidden\n");
- return sprintf(buf, "enabled\n");
+ return sysfs_emit(buf, "forbidden\n");
+ return sysfs_emit(buf, "enabled\n");
}
static DEVICE_ATTR_RO(runtime_enabled);
@@ -552,9 +552,9 @@ static DEVICE_ATTR_RO(runtime_enabled);
static ssize_t async_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- return sprintf(buf, "%s\n",
- device_async_suspend_enabled(dev) ?
- _enabled : _disabled);
+ return sysfs_emit(buf, "%s\n",
+ device_async_suspend_enabled(dev) ?
+ _enabled : _disabled);
}
static ssize_t async_store(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index c7734914d914..5568e25d7c9c 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -42,7 +42,7 @@ static ssize_t active_time_ms_show(struct device *dev,
ktime_t active_time =
ws->active ? ktime_sub(ktime_get(), ws->last_time) : 0;
- return sprintf(buf, "%lld\n", ktime_to_ms(active_time));
+ return sysfs_emit(buf, "%lld\n", ktime_to_ms(active_time));
}
static DEVICE_ATTR_RO(active_time_ms);
@@ -57,7 +57,7 @@ static ssize_t total_time_ms_show(struct device *dev,
active_time = ktime_sub(ktime_get(), ws->last_time);
total_time = ktime_add(total_time, active_time);
}
- return sprintf(buf, "%lld\n", ktime_to_ms(total_time));
+ return sysfs_emit(buf, "%lld\n", ktime_to_ms(total_time));
}
static DEVICE_ATTR_RO(total_time_ms);
@@ -73,7 +73,7 @@ static ssize_t max_time_ms_show(struct device *dev,
if (active_time > max_time)
max_time = active_time;
}
- return sprintf(buf, "%lld\n", ktime_to_ms(max_time));
+ return sysfs_emit(buf, "%lld\n", ktime_to_ms(max_time));
}
static DEVICE_ATTR_RO(max_time_ms);
@@ -82,7 +82,7 @@ static ssize_t last_change_ms_show(struct device *dev,
{
struct wakeup_source *ws = dev_get_drvdata(dev);
- return sprintf(buf, "%lld\n", ktime_to_ms(ws->last_time));
+ return sysfs_emit(buf, "%lld\n", ktime_to_ms(ws->last_time));
}
static DEVICE_ATTR_RO(last_change_ms);
@@ -91,7 +91,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
{
struct wakeup_source *ws = dev_get_drvdata(dev);
- return sprintf(buf, "%s\n", ws->name);
+ return sysfs_emit(buf, "%s\n", ws->name);
}
static DEVICE_ATTR_RO(name);
@@ -106,7 +106,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev,
prevent_sleep_time = ktime_add(prevent_sleep_time,
ktime_sub(ktime_get(), ws->start_prevent_time));
}
- return sprintf(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
+ return sysfs_emit(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
}
static DEVICE_ATTR_RO(prevent_suspend_time_ms);
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index a5bae551167d..fc6536a7eac6 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -76,15 +76,15 @@ static ssize_t soc_info_get(struct device *dev,
struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
if (attr == &dev_attr_machine)
- return sprintf(buf, "%s\n", soc_dev->attr->machine);
+ return sysfs_emit(buf, "%s\n", soc_dev->attr->machine);
if (attr == &dev_attr_family)
- return sprintf(buf, "%s\n", soc_dev->attr->family);
+ return sysfs_emit(buf, "%s\n", soc_dev->attr->family);
if (attr == &dev_attr_revision)
- return sprintf(buf, "%s\n", soc_dev->attr->revision);
+ return sysfs_emit(buf, "%s\n", soc_dev->attr->revision);
if (attr == &dev_attr_serial_number)
- return sprintf(buf, "%s\n", soc_dev->attr->serial_number);
+ return sysfs_emit(buf, "%s\n", soc_dev->attr->serial_number);
if (attr == &dev_attr_soc_id)
- return sprintf(buf, "%s\n", soc_dev->attr->soc_id);
+ return sysfs_emit(buf, "%s\n", soc_dev->attr->soc_id);
return -EINVAL;
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 3/8] drivers core: Remove strcat uses around sysfs_emit and neaten
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
2020-09-16 20:40 ` [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output Joe Perches
2020-09-16 20:40 ` [PATCH V3 2/8] drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-16 20:40 ` [PATCH V3 4/8] drivers core: Reindent a couple uses around sysfs_emit Joe Perches
` (4 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Len Brown, Pavel Machek
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-pm
strcat is no longer necessary for sysfs_emit and sysfs_emit_at uses.
Convert the strcat uses to sysfs_emit calls and neaten other block
uses of direct returns to use an intermediate const char *.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/cacheinfo.c | 25 +++++++++++++-------
drivers/base/core.c | 10 ++++----
drivers/base/memory.c | 47 ++++++++++++++++++--------------------
drivers/base/power/sysfs.c | 23 +++++++++++--------
4 files changed, 58 insertions(+), 47 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 6a8c2b5881be..96f8af414a48 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -404,17 +404,23 @@ static ssize_t type_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ const char *output;
switch (this_leaf->type) {
case CACHE_TYPE_DATA:
- return sysfs_emit(buf, "Data\n");
+ output = "Data";
+ break;
case CACHE_TYPE_INST:
- return sysfs_emit(buf, "Instruction\n");
+ output = "Instruction";
+ break;
case CACHE_TYPE_UNIFIED:
- return sysfs_emit(buf, "Unified\n");
+ output = "Unified";
+ break;
default:
return -EINVAL;
}
+
+ return sysfs_emit(buf, "%s\n", output);
}
static ssize_t allocation_policy_show(struct device *dev,
@@ -422,15 +428,18 @@ static ssize_t allocation_policy_show(struct device *dev,
{
struct cacheinfo *this_leaf = dev_get_drvdata(dev);
unsigned int ci_attr = this_leaf->attributes;
- int n = 0;
+ const char *output;
if ((ci_attr & CACHE_READ_ALLOCATE) && (ci_attr & CACHE_WRITE_ALLOCATE))
- n = sysfs_emit(buf, "ReadWriteAllocate\n");
+ output = "ReadWriteAllocate";
else if (ci_attr & CACHE_READ_ALLOCATE)
- n = sysfs_emit(buf, "ReadAllocate\n");
+ output = "ReadAllocate";
else if (ci_attr & CACHE_WRITE_ALLOCATE)
- n = sysfs_emit(buf, "WriteAllocate\n");
- return n;
+ output = "WriteAllocate";
+ else
+ return 0;
+
+ return sysfs_emit(buf, "%s\n", output);
}
static ssize_t write_policy_show(struct device *dev,
diff --git a/drivers/base/core.c b/drivers/base/core.c
index e17e8e941462..c5d2cc8bb8b0 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -267,16 +267,16 @@ static ssize_t auto_remove_on_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct device_link *link = to_devlink(dev);
- char *str;
+ const char *output;
if (link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
- str = "supplier unbind";
+ output = "supplier unbind";
else if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER)
- str = "consumer unbind";
+ output = "consumer unbind";
else
- str = "never";
+ output = "never";
- return sysfs_emit(buf, "%s\n", str);
+ return sysfs_emit(buf, "%s\n", output);
}
static DEVICE_ATTR_RO(auto_remove_on);
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index f5c0b64df2e5..7abf9eb8bff0 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -139,7 +139,7 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct memory_block *mem = to_memory_block(dev);
- ssize_t len = 0;
+ const char *output;
/*
* We can probably put these states in a nice little array
@@ -147,22 +147,20 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
*/
switch (mem->state) {
case MEM_ONLINE:
- len = sysfs_emit(buf, "online\n");
+ output = "online";
break;
case MEM_OFFLINE:
- len = sysfs_emit(buf, "offline\n");
+ output = "offline";
break;
case MEM_GOING_OFFLINE:
- len = sysfs_emit(buf, "going-offline\n");
+ output = "going-offline";
break;
default:
- len = sysfs_emit(buf, "ERROR-UNKNOWN-%ld\n",
- mem->state);
WARN_ON(1);
- break;
+ return sysfs_emit(buf, "ERROR-UNKNOWN-%ld\n", mem->state);
}
- return len;
+ return sysfs_emit(buf, "%s\n", output);
}
int memory_notify(unsigned long val, void *v)
@@ -307,17 +305,16 @@ static ssize_t phys_device_show(struct device *dev,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-static void print_allowed_zone(char *buf, int nid, unsigned long start_pfn,
- unsigned long nr_pages, int online_type,
- struct zone *default_zone)
+static int print_allowed_zone(char *buf, int len, int nid,
+ unsigned long start_pfn, unsigned long nr_pages,
+ int online_type, struct zone *default_zone)
{
struct zone *zone;
zone = zone_for_pfn_range(online_type, nid, start_pfn, nr_pages);
- if (zone != default_zone) {
- strcat(buf, " ");
- strcat(buf, zone->name);
- }
+ if (zone == default_zone)
+ return 0;
+ return sysfs_emit_at(buf, len, " %s", zone->name);
}
static ssize_t valid_zones_show(struct device *dev,
@@ -327,6 +324,7 @@ static ssize_t valid_zones_show(struct device *dev,
unsigned long start_pfn = section_nr_to_pfn(mem->start_section_nr);
unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
struct zone *default_zone;
+ int len = 0;
int nid;
/*
@@ -341,24 +339,23 @@ static ssize_t valid_zones_show(struct device *dev,
default_zone = test_pages_in_a_zone(start_pfn,
start_pfn + nr_pages);
if (!default_zone)
- return sysfs_emit(buf, "none\n");
- strcat(buf, default_zone->name);
+ return sysfs_emit(buf, "%s\n", "none");
+ len += sysfs_emit_at(buf, len, "%s", default_zone->name);
goto out;
}
nid = mem->nid;
default_zone = zone_for_pfn_range(MMOP_ONLINE, nid, start_pfn,
nr_pages);
- strcat(buf, default_zone->name);
- print_allowed_zone(buf, nid, start_pfn, nr_pages, MMOP_ONLINE_KERNEL,
- default_zone);
- print_allowed_zone(buf, nid, start_pfn, nr_pages, MMOP_ONLINE_MOVABLE,
- default_zone);
+ len += sysfs_emit_at(buf, len, "%s", default_zone->name);
+ len += print_allowed_zone(buf, len, nid, start_pfn, nr_pages,
+ MMOP_ONLINE_KERNEL, default_zone);
+ len += print_allowed_zone(buf, len, nid, start_pfn, nr_pages,
+ MMOP_ONLINE_MOVABLE, default_zone);
out:
- strcat(buf, "\n");
-
- return strlen(buf);
+ len += sysfs_emit_at(buf, len, "%s", "\n");
+ return len;
}
static DEVICE_ATTR_RO(valid_zones);
#endif
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 4276b792d0aa..61e16786f6c5 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -255,9 +255,9 @@ static ssize_t pm_qos_latency_tolerance_us_show(struct device *dev,
s32 value = dev_pm_qos_get_user_latency_tolerance(dev);
if (value < 0)
- return sysfs_emit(buf, "auto\n");
+ return sysfs_emit(buf, "%s\n", "auto");
if (value == PM_QOS_LATENCY_ANY)
- return sysfs_emit(buf, "any\n");
+ return sysfs_emit(buf, "%s\n", "any");
return sysfs_emit(buf, "%d\n", value);
}
@@ -538,13 +538,18 @@ static DEVICE_ATTR_RO(runtime_active_kids);
static ssize_t runtime_enabled_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- if (dev->power.disable_depth && (dev->power.runtime_auto == false))
- return sysfs_emit(buf, "disabled & forbidden\n");
- if (dev->power.disable_depth)
- return sysfs_emit(buf, "disabled\n");
- if (dev->power.runtime_auto == false)
- return sysfs_emit(buf, "forbidden\n");
- return sysfs_emit(buf, "enabled\n");
+ const char *output;
+
+ if (dev->power.disable_depth && !dev->power.runtime_auto)
+ output = "disabled & forbidden";
+ else if (dev->power.disable_depth)
+ output = "disabled";
+ else if (!dev->power.runtime_auto)
+ output = "forbidden";
+ else
+ output = "enabled";
+
+ return sysfs_emit(buf, "%s\n", output);
}
static DEVICE_ATTR_RO(runtime_enabled);
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 4/8] drivers core: Reindent a couple uses around sysfs_emit
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
` (2 preceding siblings ...)
2020-09-16 20:40 ` [PATCH V3 3/8] drivers core: Remove strcat uses around sysfs_emit and neaten Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-16 20:40 ` [PATCH V3 5/8] drivers core: Miscellaneous changes for sysfs_emit Joe Perches
` (3 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Pavel Machek, Len Brown
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-pm
Just a couple of whitespace realignment to open parenthesis for
multi-line statements.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/node.c | 4 ++--
drivers/base/power/sysfs.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index f6bde7acd5fe..b2dcca375da6 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -386,9 +386,9 @@ static ssize_t node_read_meminfo(struct device *dev,
nid, K(i.freeram),
nid, K(i.totalram - i.freeram),
nid, K(node_page_state(pgdat, NR_ACTIVE_ANON) +
- node_page_state(pgdat, NR_ACTIVE_FILE)),
+ node_page_state(pgdat, NR_ACTIVE_FILE)),
nid, K(node_page_state(pgdat, NR_INACTIVE_ANON) +
- node_page_state(pgdat, NR_INACTIVE_FILE)),
+ node_page_state(pgdat, NR_INACTIVE_FILE)),
nid, K(node_page_state(pgdat, NR_ACTIVE_ANON)),
nid, K(node_page_state(pgdat, NR_INACTIVE_ANON)),
nid, K(node_page_state(pgdat, NR_ACTIVE_FILE)),
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 61e16786f6c5..e05207abb99e 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -102,7 +102,7 @@ static ssize_t control_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
return sysfs_emit(buf, "%s\n",
- dev->power.runtime_auto ? ctrl_auto : ctrl_on);
+ dev->power.runtime_auto ? ctrl_auto : ctrl_on);
}
static ssize_t control_store(struct device * dev, struct device_attribute *attr,
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 5/8] drivers core: Miscellaneous changes for sysfs_emit
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
` (3 preceding siblings ...)
2020-09-16 20:40 ` [PATCH V3 4/8] drivers core: Reindent a couple uses around sysfs_emit Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-16 20:40 ` [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit Joe Perches
` (2 subsequent siblings)
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Johannes Berg,
Luis Chamberlain, Pavel Machek, Len Brown
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-pm
Change additional instances that could use sysfs_emit and sysfs_emit_at
that the coccinelle script could not convert.
o macros creating show functions with ## concatenation
o unbound sprintf uses with buf+len for start of output to sysfs_emit_at
o returns with ?: tests and sprintf to sysfs_emit
o sysfs output with struct class * not struct device * arguments
Miscellanea:
o remove unnecessary initializations around these changes
o consistently use int len for return length of show functions
o use octal permissions and not S_<FOO>
o rename a few show function names so DEVICE_ATTR_<FOO> can be used
o use DEVICE_ATTR_ADMIN_RO where appropriate
o consistently use const char *output for strings
o checkpatch/style neatening
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/bus.c | 2 +-
drivers/base/cacheinfo.c | 2 +-
drivers/base/class.c | 2 +-
drivers/base/core.c | 34 +--
drivers/base/cpu.c | 62 +++---
drivers/base/dd.c | 1 +
drivers/base/devcoredump.c | 2 +-
drivers/base/firmware_loader/fallback.c | 2 +-
drivers/base/memory.c | 5 +-
drivers/base/node.c | 268 ++++++++++++------------
drivers/base/platform.c | 13 +-
drivers/base/power/sysfs.c | 103 +++++----
drivers/base/power/wakeup_stats.c | 5 +-
drivers/base/soc.c | 64 +++---
drivers/base/topology.c | 10 +-
15 files changed, 308 insertions(+), 267 deletions(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 886e9054999a..a9c23ecebc7c 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -229,7 +229,7 @@ static DRIVER_ATTR_IGNORE_LOCKDEP(bind, S_IWUSR, NULL, bind_store);
static ssize_t drivers_autoprobe_show(struct bus_type *bus, char *buf)
{
- return sprintf(buf, "%d\n", bus->p->drivers_autoprobe);
+ return sysfs_emit(buf, "%d\n", bus->p->drivers_autoprobe);
}
static ssize_t drivers_autoprobe_store(struct bus_type *bus,
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 96f8af414a48..4946647bd985 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -362,7 +362,7 @@ static ssize_t file_name##_show(struct device *dev, \
struct device_attribute *attr, char *buf) \
{ \
struct cacheinfo *this_leaf = dev_get_drvdata(dev); \
- return sprintf(buf, "%u\n", this_leaf->object); \
+ return sysfs_emit(buf, "%u\n", this_leaf->object); \
}
show_one(id, id);
diff --git a/drivers/base/class.c b/drivers/base/class.c
index bcd410e6d70a..c3451481194e 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -478,7 +478,7 @@ ssize_t show_class_attr_string(struct class *class,
struct class_attribute_string *cs;
cs = container_of(attr, struct class_attribute_string, attr);
- return snprintf(buf, PAGE_SIZE, "%s\n", cs->str);
+ return sysfs_emit(buf, "%s\n", cs->str);
}
EXPORT_SYMBOL_GPL(show_class_attr_string);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index c5d2cc8bb8b0..3a0701261ff2 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -239,27 +239,35 @@ void device_pm_move_to_tail(struct device *dev)
#define to_devlink(dev) container_of((dev), struct device_link, link_dev)
static ssize_t status_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
- char *status;
+ const char *output;
switch (to_devlink(dev)->status) {
case DL_STATE_NONE:
- status = "not tracked"; break;
+ output = "not tracked";
+ break;
case DL_STATE_DORMANT:
- status = "dormant"; break;
+ output = "dormant";
+ break;
case DL_STATE_AVAILABLE:
- status = "available"; break;
+ output = "available";
+ break;
case DL_STATE_CONSUMER_PROBE:
- status = "consumer probing"; break;
+ output = "consumer probing";
+ break;
case DL_STATE_ACTIVE:
- status = "active"; break;
+ output = "active";
+ break;
case DL_STATE_SUPPLIER_UNBIND:
- status = "supplier unbinding"; break;
+ output = "supplier unbinding";
+ break;
default:
- status = "unknown"; break;
+ output = "unknown";
+ break;
}
- return sysfs_emit(buf, "%s\n", status);
+
+ return sysfs_emit(buf, "%s\n", output);
}
static DEVICE_ATTR_RO(status);
@@ -1933,7 +1941,7 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,
struct kset *kset;
struct kobj_uevent_env *env = NULL;
int i;
- size_t count = 0;
+ int len = 0;
int retval;
/* search the kset, the device belongs to */
@@ -1963,10 +1971,10 @@ static ssize_t uevent_show(struct device *dev, struct device_attribute *attr,
/* copy keys to file */
for (i = 0; i < env->envp_idx; i++)
- count += sprintf(&buf[count], "%s\n", env->envp[i]);
+ len += sysfs_emit_at(buf, len, "%s\n", env->envp[i]);
out:
kfree(env);
- return count;
+ return len;
}
static ssize_t uevent_store(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 232f8146a8c4..8f1d6569564c 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -139,11 +139,11 @@ EXPORT_SYMBOL_GPL(cpu_subsys);
#ifdef CONFIG_KEXEC
#include <linux/kexec.h>
-static ssize_t show_crash_notes(struct device *dev, struct device_attribute *attr,
+static ssize_t crash_notes_show(struct device *dev,
+ struct device_attribute *attr,
char *buf)
{
struct cpu *cpu = container_of(dev, struct cpu, dev);
- ssize_t rc;
unsigned long long addr;
int cpunum;
@@ -156,21 +156,18 @@ static ssize_t show_crash_notes(struct device *dev, struct device_attribute *att
* operation should be safe. No locking required.
*/
addr = per_cpu_ptr_to_phys(per_cpu_ptr(crash_notes, cpunum));
- rc = sysfs_emit(buf, "%Lx\n", addr);
- return rc;
+
+ return sysfs_emit(buf, "%llx\n", addr);
}
-static DEVICE_ATTR(crash_notes, 0400, show_crash_notes, NULL);
+static DEVICE_ATTR_ADMIN_RO(crash_notes);
-static ssize_t show_crash_notes_size(struct device *dev,
+static ssize_t crash_notes_size_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- ssize_t rc;
-
- rc = sysfs_emit(buf, "%zu\n", sizeof(note_buf_t));
- return rc;
+ return sysfs_emit(buf, "%zu\n", sizeof(note_buf_t));
}
-static DEVICE_ATTR(crash_notes_size, 0400, show_crash_notes_size, NULL);
+static DEVICE_ATTR_ADMIN_RO(crash_notes_size);
static struct attribute *crash_note_cpu_attrs[] = {
&dev_attr_crash_notes.attr,
@@ -241,37 +238,37 @@ unsigned int total_cpus;
static ssize_t print_cpus_offline(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n = 0, len = PAGE_SIZE-2;
+ int len = 0;
cpumask_var_t offline;
/* display offline cpus < nr_cpu_ids */
if (!alloc_cpumask_var(&offline, GFP_KERNEL))
return -ENOMEM;
cpumask_andnot(offline, cpu_possible_mask, cpu_online_mask);
- n = scnprintf(buf, len, "%*pbl", cpumask_pr_args(offline));
+ len += sysfs_emit_at(buf, len, "%*pbl", cpumask_pr_args(offline));
free_cpumask_var(offline);
/* display offline cpus >= nr_cpu_ids */
if (total_cpus && nr_cpu_ids < total_cpus) {
- if (n && n < len)
- buf[n++] = ',';
+ len += sysfs_emit_at(buf, len, ",");
if (nr_cpu_ids == total_cpus-1)
- n += scnprintf(&buf[n], len - n, "%u", nr_cpu_ids);
+ len += sysfs_emit_at(buf, len, "%u", nr_cpu_ids);
else
- n += scnprintf(&buf[n], len - n, "%u-%d",
- nr_cpu_ids, total_cpus-1);
+ len += sysfs_emit_at(buf, len, "%u-%d",
+ nr_cpu_ids, total_cpus - 1);
}
- n += scnprintf(&buf[n], len - n, "\n");
- return n;
+ len += sysfs_emit_at(buf, len, "\n");
+
+ return len;
}
static DEVICE_ATTR(offline, 0444, print_cpus_offline, NULL);
static ssize_t print_cpus_isolated(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n;
+ int len;
cpumask_var_t isolated;
if (!alloc_cpumask_var(&isolated, GFP_KERNEL))
@@ -279,17 +276,17 @@ static ssize_t print_cpus_isolated(struct device *dev,
cpumask_andnot(isolated, cpu_possible_mask,
housekeeping_cpumask(HK_FLAG_DOMAIN));
- n = sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(isolated));
+ len = sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(isolated));
free_cpumask_var(isolated);
- return n;
+ return len;
}
static DEVICE_ATTR(isolated, 0444, print_cpus_isolated, NULL);
#ifdef CONFIG_NO_HZ_FULL
static ssize_t print_cpus_nohz_full(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(tick_nohz_full_mask));
}
@@ -320,22 +317,23 @@ static ssize_t print_cpu_modalias(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- ssize_t n;
+ int len = 0;
u32 i;
- n = sysfs_emit(buf, "cpu:type:" CPU_FEATURE_TYPEFMT ":feature:",
- CPU_FEATURE_TYPEVAL);
+ len += sysfs_emit_at(buf, len,
+ "cpu:type:" CPU_FEATURE_TYPEFMT ":feature:",
+ CPU_FEATURE_TYPEVAL);
for (i = 0; i < MAX_CPU_FEATURES; i++)
if (cpu_have_feature(i)) {
- if (PAGE_SIZE < n + sizeof(",XXXX\n")) {
+ if (len + sizeof(",XXXX\n") >= PAGE_SIZE) {
WARN(1, "CPU features overflow page\n");
break;
}
- n += sprintf(&buf[n], ",%04X", i);
+ len += sysfs_emit_at(buf, len, ",%04X", i);
}
- buf[n++] = '\n';
- return n;
+ len += sysfs_emit_at(buf, len, "\n");
+ return len;
}
static int cpu_uevent(struct device *dev, struct kobj_uevent_env *env)
@@ -557,7 +555,7 @@ ssize_t __weak cpu_show_tsx_async_abort(struct device *dev,
}
ssize_t __weak cpu_show_itlb_multihit(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
return sysfs_emit(buf, "Not affected\n");
}
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 123a1c3e9f18..b52d69eb4e71 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -486,6 +486,7 @@ static ssize_t state_synced_show(struct device *dev,
device_lock(dev);
val = dev->state_synced;
device_unlock(dev);
+
return sysfs_emit(buf, "%u\n", val);
}
static DEVICE_ATTR_RO(state_synced);
diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index e42d0b514384..9243468e2c99 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -123,7 +123,7 @@ static int devcd_free(struct device *dev, void *data)
static ssize_t disabled_show(struct class *class, struct class_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", devcd_disabled);
+ return sysfs_emit(buf, "%d\n", devcd_disabled);
}
static ssize_t disabled_store(struct class *class, struct class_attribute *attr,
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 7e9598a1577a..d60e6d8c967c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -124,7 +124,7 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom)
static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
char *buf)
{
- return sprintf(buf, "%d\n", __firmware_loading_timeout());
+ return sysfs_emit(buf, "%d\n", __firmware_loading_timeout());
}
/**
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 7abf9eb8bff0..eef4ffb6122c 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -119,6 +119,7 @@ static ssize_t phys_index_show(struct device *dev,
unsigned long phys_index;
phys_index = mem->start_section_nr / sections_per_block;
+
return sysfs_emit(buf, "%08lx\n", phys_index);
}
@@ -301,6 +302,7 @@ static ssize_t phys_device_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct memory_block *mem = to_memory_block(dev);
+
return sysfs_emit(buf, "%d\n", mem->phys_device);
}
@@ -314,6 +316,7 @@ static int print_allowed_zone(char *buf, int len, int nid,
zone = zone_for_pfn_range(online_type, nid, start_pfn, nr_pages);
if (zone == default_zone)
return 0;
+
return sysfs_emit_at(buf, len, " %s", zone->name);
}
@@ -354,7 +357,7 @@ static ssize_t valid_zones_show(struct device *dev,
len += print_allowed_zone(buf, len, nid, start_pfn, nr_pages,
MMOP_ONLINE_MOVABLE, default_zone);
out:
- len += sysfs_emit_at(buf, len, "%s", "\n");
+ len += sysfs_emit_at(buf, len, "\n");
return len;
}
static DEVICE_ATTR_RO(valid_zones);
diff --git a/drivers/base/node.c b/drivers/base/node.c
index b2dcca375da6..b33526a9fcfc 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -46,19 +46,23 @@ static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
return n;
}
-static inline ssize_t node_read_cpumask(struct device *dev,
- struct device_attribute *attr, char *buf)
+static inline ssize_t cpumap_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
{
return node_read_cpumap(dev, false, buf);
}
-static inline ssize_t node_read_cpulist(struct device *dev,
- struct device_attribute *attr, char *buf)
+
+static DEVICE_ATTR_RO(cpumap);
+
+static inline ssize_t cpulist_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
{
return node_read_cpumap(dev, true, buf);
}
-static DEVICE_ATTR(cpumap, S_IRUGO, node_read_cpumask, NULL);
-static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
+static DEVICE_ATTR_RO(cpulist);
/**
* struct node_access_nodes - Access class device to hold user visible
@@ -153,13 +157,14 @@ static struct node_access_nodes *node_init_node_access(struct node *node,
}
#ifdef CONFIG_HMEM_REPORTING
-#define ACCESS_ATTR(name) \
-static ssize_t name##_show(struct device *dev, \
- struct device_attribute *attr, \
- char *buf) \
-{ \
- return sprintf(buf, "%u\n", to_access_nodes(dev)->hmem_attrs.name); \
-} \
+#define ACCESS_ATTR(name) \
+static ssize_t name##_show(struct device *dev, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ return sysfs_emit(buf, "%u\n", \
+ to_access_nodes(dev)->hmem_attrs.name); \
+} \
static DEVICE_ATTR_RO(name);
ACCESS_ATTR(read_bandwidth)
@@ -225,7 +230,8 @@ static ssize_t name##_show(struct device *dev, \
struct device_attribute *attr, \
char *buf) \
{ \
- return sprintf(buf, fmt "\n", to_cache_info(dev)->cache_attrs.name);\
+ return sysfs_emit(buf, fmt "\n", \
+ to_cache_info(dev)->cache_attrs.name); \
} \
DEVICE_ATTR_RO(name);
@@ -361,7 +367,7 @@ static void node_remove_caches(struct node *node) { }
static ssize_t node_read_meminfo(struct device *dev,
struct device_attribute *attr, char *buf)
{
- int n;
+ int len = 0;
int nid = dev->id;
struct pglist_data *pgdat = NODE_DATA(nid);
struct sysinfo i;
@@ -370,112 +376,112 @@ static ssize_t node_read_meminfo(struct device *dev,
si_meminfo_node(&i, nid);
sreclaimable = node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B);
sunreclaimable = node_page_state_pages(pgdat, NR_SLAB_UNRECLAIMABLE_B);
- n = sysfs_emit(buf,
- "Node %d MemTotal: %8lu kB\n"
- "Node %d MemFree: %8lu kB\n"
- "Node %d MemUsed: %8lu kB\n"
- "Node %d Active: %8lu kB\n"
- "Node %d Inactive: %8lu kB\n"
- "Node %d Active(anon): %8lu kB\n"
- "Node %d Inactive(anon): %8lu kB\n"
- "Node %d Active(file): %8lu kB\n"
- "Node %d Inactive(file): %8lu kB\n"
- "Node %d Unevictable: %8lu kB\n"
- "Node %d Mlocked: %8lu kB\n",
- nid, K(i.totalram),
- nid, K(i.freeram),
- nid, K(i.totalram - i.freeram),
- nid, K(node_page_state(pgdat, NR_ACTIVE_ANON) +
- node_page_state(pgdat, NR_ACTIVE_FILE)),
- nid, K(node_page_state(pgdat, NR_INACTIVE_ANON) +
- node_page_state(pgdat, NR_INACTIVE_FILE)),
- nid, K(node_page_state(pgdat, NR_ACTIVE_ANON)),
- nid, K(node_page_state(pgdat, NR_INACTIVE_ANON)),
- nid, K(node_page_state(pgdat, NR_ACTIVE_FILE)),
- nid, K(node_page_state(pgdat, NR_INACTIVE_FILE)),
- nid, K(node_page_state(pgdat, NR_UNEVICTABLE)),
- nid, K(sum_zone_node_page_state(nid, NR_MLOCK)));
+ len = sysfs_emit_at(buf, len,
+ "Node %d MemTotal: %8lu kB\n"
+ "Node %d MemFree: %8lu kB\n"
+ "Node %d MemUsed: %8lu kB\n"
+ "Node %d Active: %8lu kB\n"
+ "Node %d Inactive: %8lu kB\n"
+ "Node %d Active(anon): %8lu kB\n"
+ "Node %d Inactive(anon): %8lu kB\n"
+ "Node %d Active(file): %8lu kB\n"
+ "Node %d Inactive(file): %8lu kB\n"
+ "Node %d Unevictable: %8lu kB\n"
+ "Node %d Mlocked: %8lu kB\n",
+ nid, K(i.totalram),
+ nid, K(i.freeram),
+ nid, K(i.totalram - i.freeram),
+ nid, K(node_page_state(pgdat, NR_ACTIVE_ANON) +
+ node_page_state(pgdat, NR_ACTIVE_FILE)),
+ nid, K(node_page_state(pgdat, NR_INACTIVE_ANON) +
+ node_page_state(pgdat, NR_INACTIVE_FILE)),
+ nid, K(node_page_state(pgdat, NR_ACTIVE_ANON)),
+ nid, K(node_page_state(pgdat, NR_INACTIVE_ANON)),
+ nid, K(node_page_state(pgdat, NR_ACTIVE_FILE)),
+ nid, K(node_page_state(pgdat, NR_INACTIVE_FILE)),
+ nid, K(node_page_state(pgdat, NR_UNEVICTABLE)),
+ nid, K(sum_zone_node_page_state(nid, NR_MLOCK)));
#ifdef CONFIG_HIGHMEM
- n += sprintf(buf + n,
- "Node %d HighTotal: %8lu kB\n"
- "Node %d HighFree: %8lu kB\n"
- "Node %d LowTotal: %8lu kB\n"
- "Node %d LowFree: %8lu kB\n",
- nid, K(i.totalhigh),
- nid, K(i.freehigh),
- nid, K(i.totalram - i.totalhigh),
- nid, K(i.freeram - i.freehigh));
+ len += sysfs_emit_at(buf, len,
+ "Node %d HighTotal: %8lu kB\n"
+ "Node %d HighFree: %8lu kB\n"
+ "Node %d LowTotal: %8lu kB\n"
+ "Node %d LowFree: %8lu kB\n",
+ nid, K(i.totalhigh),
+ nid, K(i.freehigh),
+ nid, K(i.totalram - i.totalhigh),
+ nid, K(i.freeram - i.freehigh));
#endif
- n += sprintf(buf + n,
- "Node %d Dirty: %8lu kB\n"
- "Node %d Writeback: %8lu kB\n"
- "Node %d FilePages: %8lu kB\n"
- "Node %d Mapped: %8lu kB\n"
- "Node %d AnonPages: %8lu kB\n"
- "Node %d Shmem: %8lu kB\n"
- "Node %d KernelStack: %8lu kB\n"
+ len += sysfs_emit_at(buf, len,
+ "Node %d Dirty: %8lu kB\n"
+ "Node %d Writeback: %8lu kB\n"
+ "Node %d FilePages: %8lu kB\n"
+ "Node %d Mapped: %8lu kB\n"
+ "Node %d AnonPages: %8lu kB\n"
+ "Node %d Shmem: %8lu kB\n"
+ "Node %d KernelStack: %8lu kB\n"
#ifdef CONFIG_SHADOW_CALL_STACK
- "Node %d ShadowCallStack:%8lu kB\n"
+ "Node %d ShadowCallStack:%8lu kB\n"
#endif
- "Node %d PageTables: %8lu kB\n"
- "Node %d NFS_Unstable: %8lu kB\n"
- "Node %d Bounce: %8lu kB\n"
- "Node %d WritebackTmp: %8lu kB\n"
- "Node %d KReclaimable: %8lu kB\n"
- "Node %d Slab: %8lu kB\n"
- "Node %d SReclaimable: %8lu kB\n"
- "Node %d SUnreclaim: %8lu kB\n"
+ "Node %d PageTables: %8lu kB\n"
+ "Node %d NFS_Unstable: %8lu kB\n"
+ "Node %d Bounce: %8lu kB\n"
+ "Node %d WritebackTmp: %8lu kB\n"
+ "Node %d KReclaimable: %8lu kB\n"
+ "Node %d Slab: %8lu kB\n"
+ "Node %d SReclaimable: %8lu kB\n"
+ "Node %d SUnreclaim: %8lu kB\n"
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- "Node %d AnonHugePages: %8lu kB\n"
- "Node %d ShmemHugePages: %8lu kB\n"
- "Node %d ShmemPmdMapped: %8lu kB\n"
- "Node %d FileHugePages: %8lu kB\n"
- "Node %d FilePmdMapped: %8lu kB\n"
+ "Node %d AnonHugePages: %8lu kB\n"
+ "Node %d ShmemHugePages: %8lu kB\n"
+ "Node %d ShmemPmdMapped: %8lu kB\n"
+ "Node %d FileHugePages: %8lu kB\n"
+ "Node %d FilePmdMapped: %8lu kB\n"
#endif
- ,
- nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
- nid, K(node_page_state(pgdat, NR_WRITEBACK)),
- nid, K(node_page_state(pgdat, NR_FILE_PAGES)),
- nid, K(node_page_state(pgdat, NR_FILE_MAPPED)),
- nid, K(node_page_state(pgdat, NR_ANON_MAPPED)),
- nid, K(i.sharedram),
- nid, node_page_state(pgdat, NR_KERNEL_STACK_KB),
+ ,
+ nid, K(node_page_state(pgdat, NR_FILE_DIRTY)),
+ nid, K(node_page_state(pgdat, NR_WRITEBACK)),
+ nid, K(node_page_state(pgdat, NR_FILE_PAGES)),
+ nid, K(node_page_state(pgdat, NR_FILE_MAPPED)),
+ nid, K(node_page_state(pgdat, NR_ANON_MAPPED)),
+ nid, K(i.sharedram),
+ nid, node_page_state(pgdat, NR_KERNEL_STACK_KB),
#ifdef CONFIG_SHADOW_CALL_STACK
- nid, node_page_state(pgdat, NR_KERNEL_SCS_KB),
+ nid, node_page_state(pgdat, NR_KERNEL_SCS_KB),
#endif
- nid, K(sum_zone_node_page_state(nid, NR_PAGETABLE)),
- nid, 0UL,
- nid, K(sum_zone_node_page_state(nid, NR_BOUNCE)),
- nid, K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
- nid, K(sreclaimable +
- node_page_state(pgdat, NR_KERNEL_MISC_RECLAIMABLE)),
- nid, K(sreclaimable + sunreclaimable),
- nid, K(sreclaimable),
- nid, K(sunreclaimable)
+ nid, K(sum_zone_node_page_state(nid, NR_PAGETABLE)),
+ nid, 0UL,
+ nid, K(sum_zone_node_page_state(nid, NR_BOUNCE)),
+ nid, K(node_page_state(pgdat, NR_WRITEBACK_TEMP)),
+ nid, K(sreclaimable +
+ node_page_state(pgdat, NR_KERNEL_MISC_RECLAIMABLE)),
+ nid, K(sreclaimable + sunreclaimable),
+ nid, K(sreclaimable),
+ nid, K(sunreclaimable)
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- ,
- nid, K(node_page_state(pgdat, NR_ANON_THPS) *
- HPAGE_PMD_NR),
- nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
- HPAGE_PMD_NR),
- nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
- HPAGE_PMD_NR),
- nid, K(node_page_state(pgdat, NR_FILE_THPS) *
- HPAGE_PMD_NR),
- nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) *
- HPAGE_PMD_NR)
+ ,
+ nid, K(node_page_state(pgdat, NR_ANON_THPS) *
+ HPAGE_PMD_NR),
+ nid, K(node_page_state(pgdat, NR_SHMEM_THPS) *
+ HPAGE_PMD_NR),
+ nid, K(node_page_state(pgdat, NR_SHMEM_PMDMAPPED) *
+ HPAGE_PMD_NR),
+ nid, K(node_page_state(pgdat, NR_FILE_THPS) *
+ HPAGE_PMD_NR),
+ nid, K(node_page_state(pgdat, NR_FILE_PMDMAPPED) *
+ HPAGE_PMD_NR)
#endif
- );
- n += hugetlb_report_node_meminfo(nid, buf + n);
- return n;
+ );
+ len += hugetlb_report_node_meminfo(nid, buf + len);
+ return len;
}
#undef K
-static DEVICE_ATTR(meminfo, S_IRUGO, node_read_meminfo, NULL);
+static DEVICE_ATTR(meminfo, 0444, node_read_meminfo, NULL);
static ssize_t node_read_numastat(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
return sysfs_emit(buf,
"numa_hit %lu\n"
@@ -491,7 +497,7 @@ static ssize_t node_read_numastat(struct device *dev,
sum_zone_numa_state(dev->id, NUMA_LOCAL),
sum_zone_numa_state(dev->id, NUMA_OTHER));
}
-static DEVICE_ATTR(numastat, S_IRUGO, node_read_numastat, NULL);
+static DEVICE_ATTR(numastat, 0444, node_read_numastat, NULL);
static ssize_t node_read_vmstat(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -499,28 +505,31 @@ static ssize_t node_read_vmstat(struct device *dev,
int nid = dev->id;
struct pglist_data *pgdat = NODE_DATA(nid);
int i;
- int n = 0;
+ int len = 0;
for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
- n += sprintf(buf+n, "%s %lu\n", zone_stat_name(i),
- sum_zone_node_page_state(nid, i));
+ len += sysfs_emit_at(buf, len, "%s %lu\n",
+ zone_stat_name(i),
+ sum_zone_node_page_state(nid, i));
#ifdef CONFIG_NUMA
for (i = 0; i < NR_VM_NUMA_STAT_ITEMS; i++)
- n += sprintf(buf+n, "%s %lu\n", numa_stat_name(i),
- sum_zone_numa_state(nid, i));
-#endif
+ len += sysfs_emit_at(buf, len, "%s %lu\n",
+ numa_stat_name(i),
+ sum_zone_numa_state(nid, i));
+#endif
for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
- n += sprintf(buf+n, "%s %lu\n", node_stat_name(i),
- node_page_state_pages(pgdat, i));
+ len += sysfs_emit_at(buf, len, "%s %lu\n",
+ node_stat_name(i),
+ node_page_state_pages(pgdat, i));
- return n;
+ return len;
}
-static DEVICE_ATTR(vmstat, S_IRUGO, node_read_vmstat, NULL);
+static DEVICE_ATTR(vmstat, 0444, node_read_vmstat, NULL);
static ssize_t node_read_distance(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
int nid = dev->id;
int len = 0;
@@ -532,13 +541,15 @@ static ssize_t node_read_distance(struct device *dev,
*/
BUILD_BUG_ON(MAX_NUMNODES * 4 > PAGE_SIZE);
- for_each_online_node(i)
- len += sprintf(buf + len, "%s%d", i ? " " : "", node_distance(nid, i));
+ for_each_online_node(i) {
+ len += sysfs_emit_at(buf, len, "%s%d",
+ i ? " " : "", node_distance(nid, i));
+ }
- len += sprintf(buf + len, "\n");
+ len += sysfs_emit_at(buf, len, "\n");
return len;
}
-static DEVICE_ATTR(distance, S_IRUGO, node_read_distance, NULL);
+static DEVICE_ATTR(distance, 0444, node_read_distance, NULL);
static struct attribute *node_dev_attrs[] = {
&dev_attr_cpumap.attr,
@@ -979,17 +990,6 @@ void unregister_one_node(int nid)
* node states attributes
*/
-static ssize_t print_nodes_state(enum node_states state, char *buf)
-{
- int n;
-
- n = scnprintf(buf, PAGE_SIZE - 1, "%*pbl",
- nodemask_pr_args(&node_states[state]));
- buf[n++] = '\n';
- buf[n] = '\0';
- return n;
-}
-
struct node_attr {
struct device_attribute attr;
enum node_states state;
@@ -999,7 +999,9 @@ static ssize_t show_node_state(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct node_attr *na = container_of(attr, struct node_attr, attr);
- return print_nodes_state(na->state, buf);
+
+ return sysfs_emit(buf, "%*pbl\n",
+ nodemask_pr_args(&node_states[na->state]));
}
#define _NODE_ATTR(name, state) \
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f0f8cfcc3621..88aef93eb4dd 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1023,10 +1023,10 @@ EXPORT_SYMBOL_GPL(platform_unregister_drivers);
* (b) sysfs attribute lets new-style coldplug recover from hotplug events
* mishandled before system is fully running: "modprobe $(cat modalias)"
*/
-static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
- char *buf)
+static ssize_t modalias_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
- struct platform_device *pdev = to_platform_device(dev);
+ struct platform_device *pdev = to_platform_device(dev);
int len;
len = of_device_modalias(dev, buf, PAGE_SIZE);
@@ -1037,9 +1037,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
if (len != -ENODEV)
return len;
- len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
-
- return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
+ return sysfs_emit(buf, "platform:%s\n", pdev->name);
}
static DEVICE_ATTR_RO(modalias);
@@ -1086,12 +1084,13 @@ static ssize_t driver_override_show(struct device *dev,
device_lock(dev);
len = sysfs_emit(buf, "%s\n", pdev->driver_override);
device_unlock(dev);
+
return len;
}
static DEVICE_ATTR_RW(driver_override);
static ssize_t numa_node_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
return sysfs_emit(buf, "%d\n", dev_to_node(dev));
}
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index e05207abb99e..a1474fb67db9 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -122,66 +122,70 @@ static ssize_t control_store(struct device * dev, struct device_attribute *attr,
static DEVICE_ATTR_RW(control);
static ssize_t runtime_active_time_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
- int ret;
u64 tmp = pm_runtime_active_time(dev);
+
do_div(tmp, NSEC_PER_MSEC);
- ret = sysfs_emit(buf, "%llu\n", tmp);
- return ret;
+
+ return sysfs_emit(buf, "%llu\n", tmp);
}
static DEVICE_ATTR_RO(runtime_active_time);
static ssize_t runtime_suspended_time_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
- int ret;
u64 tmp = pm_runtime_suspended_time(dev);
+
do_div(tmp, NSEC_PER_MSEC);
- ret = sysfs_emit(buf, "%llu\n", tmp);
- return ret;
+
+ return sysfs_emit(buf, "%llu\n", tmp);
}
static DEVICE_ATTR_RO(runtime_suspended_time);
static ssize_t runtime_status_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr, char *buf)
{
- const char *p;
+ const char *output;
if (dev->power.runtime_error) {
- p = "error\n";
+ output = "error";
} else if (dev->power.disable_depth) {
- p = "unsupported\n";
+ output = "unsupported";
} else {
switch (dev->power.runtime_status) {
case RPM_SUSPENDED:
- p = "suspended\n";
+ output = "suspended";
break;
case RPM_SUSPENDING:
- p = "suspending\n";
+ output = "suspending";
break;
case RPM_RESUMING:
- p = "resuming\n";
+ output = "resuming";
break;
case RPM_ACTIVE:
- p = "active\n";
+ output = "active";
break;
default:
return -EIO;
}
}
- return sysfs_emit(buf, p);
+ return sysfs_emit(buf, "%s\n", output);
}
static DEVICE_ATTR_RO(runtime_status);
static ssize_t autosuspend_delay_ms_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+ struct device_attribute *attr,
+ char *buf)
{
if (!dev->power.use_autosuspend)
return -EIO;
+
return sysfs_emit(buf, "%d\n", dev->power.autosuspend_delay);
}
@@ -345,7 +349,7 @@ static DEVICE_ATTR_RW(wakeup);
static ssize_t wakeup_count_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- unsigned long count = 0;
+ unsigned long count;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -354,7 +358,10 @@ static ssize_t wakeup_count_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lu\n", count) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lu\n", count);
}
static DEVICE_ATTR_RO(wakeup_count);
@@ -363,7 +370,7 @@ static ssize_t wakeup_active_count_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- unsigned long count = 0;
+ unsigned long count;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -372,7 +379,10 @@ static ssize_t wakeup_active_count_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lu\n", count) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lu\n", count);
}
static DEVICE_ATTR_RO(wakeup_active_count);
@@ -381,7 +391,7 @@ static ssize_t wakeup_abort_count_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- unsigned long count = 0;
+ unsigned long count;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -390,7 +400,10 @@ static ssize_t wakeup_abort_count_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lu\n", count) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lu\n", count);
}
static DEVICE_ATTR_RO(wakeup_abort_count);
@@ -399,7 +412,7 @@ static ssize_t wakeup_expire_count_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- unsigned long count = 0;
+ unsigned long count;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -408,7 +421,10 @@ static ssize_t wakeup_expire_count_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lu\n", count) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lu\n", count);
}
static DEVICE_ATTR_RO(wakeup_expire_count);
@@ -416,7 +432,7 @@ static DEVICE_ATTR_RO(wakeup_expire_count);
static ssize_t wakeup_active_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- unsigned int active = 0;
+ unsigned int active;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -425,7 +441,10 @@ static ssize_t wakeup_active_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%u\n", active) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%u\n", active);
}
static DEVICE_ATTR_RO(wakeup_active);
@@ -434,7 +453,7 @@ static ssize_t wakeup_total_time_ms_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- s64 msec = 0;
+ s64 msec;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -443,7 +462,10 @@ static ssize_t wakeup_total_time_ms_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lld\n", msec) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lld\n", msec);
}
static DEVICE_ATTR_RO(wakeup_total_time_ms);
@@ -451,7 +473,7 @@ static DEVICE_ATTR_RO(wakeup_total_time_ms);
static ssize_t wakeup_max_time_ms_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- s64 msec = 0;
+ s64 msec;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -460,7 +482,10 @@ static ssize_t wakeup_max_time_ms_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lld\n", msec) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lld\n", msec);
}
static DEVICE_ATTR_RO(wakeup_max_time_ms);
@@ -469,7 +494,7 @@ static ssize_t wakeup_last_time_ms_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- s64 msec = 0;
+ s64 msec;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -478,7 +503,10 @@ static ssize_t wakeup_last_time_ms_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lld\n", msec) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lld\n", msec);
}
static inline int dpm_sysfs_wakeup_change_owner(struct device *dev, kuid_t kuid,
@@ -496,7 +524,7 @@ static ssize_t wakeup_prevent_sleep_time_ms_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
- s64 msec = 0;
+ s64 msec;
bool enabled = false;
spin_lock_irq(&dev->power.lock);
@@ -505,7 +533,10 @@ static ssize_t wakeup_prevent_sleep_time_ms_show(struct device *dev,
enabled = true;
}
spin_unlock_irq(&dev->power.lock);
- return enabled ? sprintf(buf, "%lld\n", msec) : sprintf(buf, "\n");
+
+ if (!enabled)
+ return sysfs_emit(buf, "\n");
+ return sysfs_emit(buf, "%lld\n", msec);
}
static DEVICE_ATTR_RO(wakeup_prevent_sleep_time_ms);
diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_stats.c
index 5568e25d7c9c..d638259b829a 100644
--- a/drivers/base/power/wakeup_stats.c
+++ b/drivers/base/power/wakeup_stats.c
@@ -26,7 +26,7 @@ static ssize_t _name##_show(struct device *dev, \
{ \
struct wakeup_source *ws = dev_get_drvdata(dev); \
\
- return sprintf(buf, "%lu\n", ws->_name); \
+ return sysfs_emit(buf, "%lu\n", ws->_name); \
} \
static DEVICE_ATTR_RO(_name)
@@ -57,6 +57,7 @@ static ssize_t total_time_ms_show(struct device *dev,
active_time = ktime_sub(ktime_get(), ws->last_time);
total_time = ktime_add(total_time, active_time);
}
+
return sysfs_emit(buf, "%lld\n", ktime_to_ms(total_time));
}
static DEVICE_ATTR_RO(total_time_ms);
@@ -73,6 +74,7 @@ static ssize_t max_time_ms_show(struct device *dev,
if (active_time > max_time)
max_time = active_time;
}
+
return sysfs_emit(buf, "%lld\n", ktime_to_ms(max_time));
}
static DEVICE_ATTR_RO(max_time_ms);
@@ -106,6 +108,7 @@ static ssize_t prevent_suspend_time_ms_show(struct device *dev,
prevent_sleep_time = ktime_add(prevent_sleep_time,
ktime_sub(ktime_get(), ws->start_prevent_time));
}
+
return sysfs_emit(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
}
static DEVICE_ATTR_RO(prevent_suspend_time_ms);
diff --git a/drivers/base/soc.c b/drivers/base/soc.c
index fc6536a7eac6..d34609bb7386 100644
--- a/drivers/base/soc.c
+++ b/drivers/base/soc.c
@@ -17,9 +17,9 @@
static DEFINE_IDA(soc_ida);
-static ssize_t soc_info_get(struct device *dev,
- struct device_attribute *attr,
- char *buf);
+/* Prototype to allow declarations of DEVICE_ATTR(<foo>) before soc_info_show */
+static ssize_t soc_info_show(struct device *dev, struct device_attribute *attr,
+ char *buf);
struct soc_device {
struct device dev;
@@ -31,11 +31,11 @@ static struct bus_type soc_bus_type = {
.name = "soc",
};
-static DEVICE_ATTR(machine, S_IRUGO, soc_info_get, NULL);
-static DEVICE_ATTR(family, S_IRUGO, soc_info_get, NULL);
-static DEVICE_ATTR(serial_number, S_IRUGO, soc_info_get, NULL);
-static DEVICE_ATTR(soc_id, S_IRUGO, soc_info_get, NULL);
-static DEVICE_ATTR(revision, S_IRUGO, soc_info_get, NULL);
+static DEVICE_ATTR(machine, 0444, soc_info_show, NULL);
+static DEVICE_ATTR(family, 0444, soc_info_show, NULL);
+static DEVICE_ATTR(serial_number, 0444, soc_info_show, NULL);
+static DEVICE_ATTR(soc_id, 0444, soc_info_show, NULL);
+static DEVICE_ATTR(revision, 0444, soc_info_show, NULL);
struct device *soc_device_to_device(struct soc_device *soc_dev)
{
@@ -49,45 +49,41 @@ static umode_t soc_attribute_mode(struct kobject *kobj,
struct device *dev = kobj_to_dev(kobj);
struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
- if ((attr == &dev_attr_machine.attr)
- && (soc_dev->attr->machine != NULL))
+ if ((attr == &dev_attr_machine.attr) && soc_dev->attr->machine)
return attr->mode;
- if ((attr == &dev_attr_family.attr)
- && (soc_dev->attr->family != NULL))
+ if ((attr == &dev_attr_family.attr) && soc_dev->attr->family)
return attr->mode;
- if ((attr == &dev_attr_revision.attr)
- && (soc_dev->attr->revision != NULL))
+ if ((attr == &dev_attr_revision.attr) && soc_dev->attr->revision)
return attr->mode;
- if ((attr == &dev_attr_serial_number.attr)
- && (soc_dev->attr->serial_number != NULL))
+ if ((attr == &dev_attr_serial_number.attr) && soc_dev->attr->serial_number)
return attr->mode;
- if ((attr == &dev_attr_soc_id.attr)
- && (soc_dev->attr->soc_id != NULL))
+ if ((attr == &dev_attr_soc_id.attr) && soc_dev->attr->soc_id)
return attr->mode;
- /* Unknown or unfilled attribute. */
+ /* Unknown or unfilled attribute */
return 0;
}
-static ssize_t soc_info_get(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+static ssize_t soc_info_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
{
struct soc_device *soc_dev = container_of(dev, struct soc_device, dev);
+ const char *output;
if (attr == &dev_attr_machine)
- return sysfs_emit(buf, "%s\n", soc_dev->attr->machine);
- if (attr == &dev_attr_family)
- return sysfs_emit(buf, "%s\n", soc_dev->attr->family);
- if (attr == &dev_attr_revision)
- return sysfs_emit(buf, "%s\n", soc_dev->attr->revision);
- if (attr == &dev_attr_serial_number)
- return sysfs_emit(buf, "%s\n", soc_dev->attr->serial_number);
- if (attr == &dev_attr_soc_id)
- return sysfs_emit(buf, "%s\n", soc_dev->attr->soc_id);
-
- return -EINVAL;
-
+ output = soc_dev->attr->machine;
+ else if (attr == &dev_attr_family)
+ output = soc_dev->attr->family;
+ else if (attr == &dev_attr_revision)
+ output = soc_dev->attr->revision;
+ else if (attr == &dev_attr_serial_number)
+ output = soc_dev->attr->serial_number;
+ else if (attr == &dev_attr_soc_id)
+ output = soc_dev->attr->soc_id;
+ else
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", output);
}
static struct attribute *soc_attr[] = {
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index ad8d33c6077b..4d254fcc93d1 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -14,11 +14,11 @@
#include <linux/hardirq.h>
#include <linux/topology.h>
-#define define_id_show_func(name) \
-static ssize_t name##_show(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- return sprintf(buf, "%d\n", topology_##name(dev->id)); \
+#define define_id_show_func(name) \
+static ssize_t name##_show(struct device *dev, \
+ struct device_attribute *attr, char *buf) \
+{ \
+ return sysfs_emit(buf, "%d\n", topology_##name(dev->id)); \
}
#define define_siblings_show_map(name, mask) \
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
` (4 preceding siblings ...)
2020-09-16 20:40 ` [PATCH V3 5/8] drivers core: Miscellaneous changes for sysfs_emit Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-18 20:29 ` Mike Kravetz
2020-09-19 6:22 ` Greg Kroah-Hartman
2020-09-16 20:40 ` [PATCH V3 7/8] drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show Joe Perches
2020-09-16 20:40 ` [PATCH V3 8/8] drivers core: node: Use a more typical macro definition style for ACCESS_ATTR Joe Perches
7 siblings, 2 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki, Mike Kravetz, Andrew Morton
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-mm
Convert the unbound sprintf in hugetlb_report_node_meminfo to use
sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/node.c | 2 +-
include/linux/hugetlb.h | 4 ++--
mm/hugetlb.c | 18 ++++++++++--------
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index b33526a9fcfc..dafe03e82e7c 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -473,7 +473,7 @@ static ssize_t node_read_meminfo(struct device *dev,
HPAGE_PMD_NR)
#endif
);
- len += hugetlb_report_node_meminfo(nid, buf + len);
+ len += hugetlb_report_node_meminfo(buf, len, nid);
return len;
}
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index d5cc5f802dd4..ebca2ef02212 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -129,7 +129,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
unsigned long start, unsigned long end,
struct page *ref_page);
void hugetlb_report_meminfo(struct seq_file *);
-int hugetlb_report_node_meminfo(int, char *);
+int hugetlb_report_node_meminfo(char *buf, int len, int nid);
void hugetlb_show_meminfo(void);
unsigned long hugetlb_total_pages(void);
vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
@@ -245,7 +245,7 @@ static inline void hugetlb_report_meminfo(struct seq_file *m)
{
}
-static inline int hugetlb_report_node_meminfo(int nid, char *buf)
+static inline int hugetlb_report_node_meminfo(char *buf, int len, int nid)
{
return 0;
}
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 61469fd3ad92..fe76f8fd5a73 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3571,18 +3571,20 @@ void hugetlb_report_meminfo(struct seq_file *m)
seq_printf(m, "Hugetlb: %8lu kB\n", total / 1024);
}
-int hugetlb_report_node_meminfo(int nid, char *buf)
+int hugetlb_report_node_meminfo(char *buf, int len, int nid)
{
struct hstate *h = &default_hstate;
+
if (!hugepages_supported())
return 0;
- return sprintf(buf,
- "Node %d HugePages_Total: %5u\n"
- "Node %d HugePages_Free: %5u\n"
- "Node %d HugePages_Surp: %5u\n",
- nid, h->nr_huge_pages_node[nid],
- nid, h->free_huge_pages_node[nid],
- nid, h->surplus_huge_pages_node[nid]);
+
+ return sysfs_emit_at(buf, len,
+ "Node %d HugePages_Total: %5u\n"
+ "Node %d HugePages_Free: %5u\n"
+ "Node %d HugePages_Surp: %5u\n",
+ nid, h->nr_huge_pages_node[nid],
+ nid, h->free_huge_pages_node[nid],
+ nid, h->surplus_huge_pages_node[nid]);
}
void hugetlb_show_meminfo(void)
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 7/8] drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
` (5 preceding siblings ...)
2020-09-16 20:40 ` [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
2020-09-16 20:40 ` [PATCH V3 8/8] drivers core: node: Use a more typical macro definition style for ACCESS_ATTR Joe Perches
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel
Do not indirect the bitmap printing of these shared_cpu show functions by
using cpumap_print_to_pagebuf/bitmap_print_to_pagebuf.
Use the more typical style with the vsnprintf %*pb and %*pbl extensions
directly so there is no possible mixup about the use of offset_in_page(buf)
by bitmap_print_to_pagebuf.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/cacheinfo.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index 4946647bd985..bfc095956dd1 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -380,24 +380,22 @@ static ssize_t size_show(struct device *dev,
return sysfs_emit(buf, "%uK\n", this_leaf->size >> 10);
}
-static ssize_t shared_cpumap_show_func(struct device *dev, bool list, char *buf)
+static ssize_t shared_cpu_map_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
{
struct cacheinfo *this_leaf = dev_get_drvdata(dev);
const struct cpumask *mask = &this_leaf->shared_cpu_map;
- return cpumap_print_to_pagebuf(list, buf, mask);
-}
-
-static ssize_t shared_cpu_map_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- return shared_cpumap_show_func(dev, false, buf);
+ return sysfs_emit(buf, "%*pb\n", nr_cpu_ids, mask);
}
static ssize_t shared_cpu_list_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- return shared_cpumap_show_func(dev, true, buf);
+ struct cacheinfo *this_leaf = dev_get_drvdata(dev);
+ const struct cpumask *mask = &this_leaf->shared_cpu_map;
+
+ return sysfs_emit(buf, "%*pbl\n", nr_cpu_ids, mask);
}
static ssize_t type_show(struct device *dev,
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH V3 8/8] drivers core: node: Use a more typical macro definition style for ACCESS_ATTR
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
` (6 preceding siblings ...)
2020-09-16 20:40 ` [PATCH V3 7/8] drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show Joe Perches
@ 2020-09-16 20:40 ` Joe Perches
7 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-16 20:40 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J . Wysocki
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel
Remove the trailing semicolon from the macro and add it to its uses.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/base/node.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index dafe03e82e7c..25dbe36c0cf2 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -165,12 +165,12 @@ static ssize_t name##_show(struct device *dev, \
return sysfs_emit(buf, "%u\n", \
to_access_nodes(dev)->hmem_attrs.name); \
} \
-static DEVICE_ATTR_RO(name);
+static DEVICE_ATTR_RO(name)
-ACCESS_ATTR(read_bandwidth)
-ACCESS_ATTR(read_latency)
-ACCESS_ATTR(write_bandwidth)
-ACCESS_ATTR(write_latency)
+ACCESS_ATTR(read_bandwidth);
+ACCESS_ATTR(read_latency);
+ACCESS_ATTR(write_bandwidth);
+ACCESS_ATTR(write_latency);
static struct attribute *access_attrs[] = {
&dev_attr_read_bandwidth.attr,
--
2.26.0
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-16 20:40 ` [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit Joe Perches
@ 2020-09-18 20:29 ` Mike Kravetz
2020-09-19 6:22 ` Greg Kroah-Hartman
1 sibling, 0 replies; 26+ messages in thread
From: Mike Kravetz @ 2020-09-18 20:29 UTC (permalink / raw)
To: Joe Perches, Greg Kroah-Hartman, Rafael J.Wysocki, Andrew Morton
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On 9/16/20 1:40 PM, Joe Perches wrote:
> Convert the unbound sprintf in hugetlb_report_node_meminfo to use
> sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-16 20:40 ` [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit Joe Perches
2020-09-18 20:29 ` Mike Kravetz
@ 2020-09-19 6:22 ` Greg Kroah-Hartman
2020-09-19 16:51 ` Joe Perches
` (2 more replies)
1 sibling, 3 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-09-19 6:22 UTC (permalink / raw)
To: Joe Perches
Cc: Rafael J . Wysocki, Mike Kravetz, Andrew Morton, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On Wed, Sep 16, 2020 at 01:40:43PM -0700, Joe Perches wrote:
> Convert the unbound sprintf in hugetlb_report_node_meminfo to use
> sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/base/node.c | 2 +-
> include/linux/hugetlb.h | 4 ++--
> mm/hugetlb.c | 18 ++++++++++--------
> 3 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index b33526a9fcfc..dafe03e82e7c 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -473,7 +473,7 @@ static ssize_t node_read_meminfo(struct device *dev,
> HPAGE_PMD_NR)
> #endif
> );
> - len += hugetlb_report_node_meminfo(nid, buf + len);
> + len += hugetlb_report_node_meminfo(buf, len, nid);
> return len;
> }
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index d5cc5f802dd4..ebca2ef02212 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -129,7 +129,7 @@ void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
> unsigned long start, unsigned long end,
> struct page *ref_page);
> void hugetlb_report_meminfo(struct seq_file *);
> -int hugetlb_report_node_meminfo(int, char *);
> +int hugetlb_report_node_meminfo(char *buf, int len, int nid);
> void hugetlb_show_meminfo(void);
> unsigned long hugetlb_total_pages(void);
> vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> @@ -245,7 +245,7 @@ static inline void hugetlb_report_meminfo(struct seq_file *m)
> {
> }
>
> -static inline int hugetlb_report_node_meminfo(int nid, char *buf)
> +static inline int hugetlb_report_node_meminfo(char *buf, int len, int nid)
> {
> return 0;
> }
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61469fd3ad92..fe76f8fd5a73 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3571,18 +3571,20 @@ void hugetlb_report_meminfo(struct seq_file *m)
> seq_printf(m, "Hugetlb: %8lu kB\n", total / 1024);
> }
>
> -int hugetlb_report_node_meminfo(int nid, char *buf)
> +int hugetlb_report_node_meminfo(char *buf, int len, int nid)
> {
> struct hstate *h = &default_hstate;
> +
> if (!hugepages_supported())
> return 0;
> - return sprintf(buf,
> - "Node %d HugePages_Total: %5u\n"
> - "Node %d HugePages_Free: %5u\n"
> - "Node %d HugePages_Surp: %5u\n",
> - nid, h->nr_huge_pages_node[nid],
> - nid, h->free_huge_pages_node[nid],
> - nid, h->surplus_huge_pages_node[nid]);
> +
> + return sysfs_emit_at(buf, len,
> + "Node %d HugePages_Total: %5u\n"
> + "Node %d HugePages_Free: %5u\n"
> + "Node %d HugePages_Surp: %5u\n",
> + nid, h->nr_huge_pages_node[nid],
> + nid, h->free_huge_pages_node[nid],
> + nid, h->surplus_huge_pages_node[nid]);
> }
That is NOT one-value-per-file, which is required for sysfs files. This
should be 3 different sysfs files.
Ugh.
But that's separate from this series, thanks for redoing this. I'll
take a look at it on Monday...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-19 6:22 ` Greg Kroah-Hartman
@ 2020-09-19 16:51 ` Joe Perches
2020-09-25 18:32 ` Joe Perches
2020-09-29 0:53 ` Joe Perches
2 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-19 16:51 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J . Wysocki, Mike Kravetz, Andrew Morton, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On Sat, 2020-09-19 at 08:22 +0200, Greg Kroah-Hartman wrote:
> I'll take a look at it on Monday...
It's decidedly not urgent/critical.
There are several logical defects all over the kernel
for these unbounded sprintfs, but I don't know of an
actual instance where the PAGE_SIZE buf is overfilled.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-19 6:22 ` Greg Kroah-Hartman
2020-09-19 16:51 ` Joe Perches
@ 2020-09-25 18:32 ` Joe Perches
2020-09-29 0:53 ` Joe Perches
2 siblings, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-09-25 18:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J . Wysocki, Mike Kravetz, Andrew Morton, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On Sat, 2020-09-19 at 08:22 +0200, Greg Kroah-Hartman wrote:
> I'll take a look at it on Monday...
Thoughts?
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-19 6:22 ` Greg Kroah-Hartman
2020-09-19 16:51 ` Joe Perches
2020-09-25 18:32 ` Joe Perches
@ 2020-09-29 0:53 ` Joe Perches
2020-09-29 4:17 ` Greg Kroah-Hartman
2 siblings, 1 reply; 26+ messages in thread
From: Joe Perches @ 2020-09-29 0:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J . Wysocki, Mike Kravetz, Andrew Morton, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On Sat, 2020-09-19 at 08:22 +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 16, 2020 at 01:40:43PM -0700, Joe Perches wrote:
> > Convert the unbound sprintf in hugetlb_report_node_meminfo to use
> > sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
[]
> I'll take a look at it on Monday...
(noting that you didn't say _which_ Monday...)
Greg are you going to take this or should I ask someone else?
The first sysfs_emit addition patch was first posted about a month
ago without much change.
I think it'd be useful to get it into -next before 5.9 is released.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit
2020-09-29 0:53 ` Joe Perches
@ 2020-09-29 4:17 ` Greg Kroah-Hartman
0 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-09-29 4:17 UTC (permalink / raw)
To: Joe Perches
Cc: Rafael J . Wysocki, Mike Kravetz, Andrew Morton, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, linux-mm
On Mon, Sep 28, 2020 at 05:53:07PM -0700, Joe Perches wrote:
> On Sat, 2020-09-19 at 08:22 +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 16, 2020 at 01:40:43PM -0700, Joe Perches wrote:
> > > Convert the unbound sprintf in hugetlb_report_node_meminfo to use
> > > sysfs_emit_at so that no possible overrun of a PAGE_SIZE buf can occur.
> []
> > I'll take a look at it on Monday...
>
> (noting that you didn't say _which_ Monday...)
True :)
> Greg are you going to take this or should I ask someone else?
>
> The first sysfs_emit addition patch was first posted about a month
> ago without much change.
>
> I think it'd be useful to get it into -next before 5.9 is released.
I'll dig into this today...
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-09-16 20:40 ` [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output Joe Perches
@ 2020-09-30 11:57 ` Greg Kroah-Hartman
2020-09-30 13:58 ` Joe Perches
2020-10-01 4:17 ` Kees Cook
0 siblings, 2 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-09-30 11:57 UTC (permalink / raw)
To: Kees Cook, Joe Perches, Rafael J . Wysocki
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel,
Jonathan Corbet, linux-doc
Kees, and Rafael, I don't know if you saw this proposal from Joe for
sysfs files, questions below:
On Wed, Sep 16, 2020 at 01:40:38PM -0700, Joe Perches wrote:
> Output defects can exist in sysfs content using sprintf and snprintf.
>
> sprintf does not know the PAGE_SIZE maximum of the temporary buffer
> used for outputting sysfs content and it's possible to overrun the
> PAGE_SIZE buffer length.
>
> Add a generic sysfs_emit function that knows that the size of the
> temporary buffer and ensures that no overrun is done.
>
> Add a generic sysfs_emit_at function that can be used in multiple
> call situations that also ensures that no overrun is done.
>
> Validate the output buffer argument to be page aligned.
> Validate the offset len argument to be within the PAGE_SIZE buf.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> Documentation/filesystems/sysfs.rst | 8 ++---
> fs/sysfs/file.c | 55 +++++++++++++++++++++++++++++
> include/linux/sysfs.h | 15 ++++++++
> 3 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/sysfs.rst b/Documentation/filesystems/sysfs.rst
> index 5a3209a4cebf..004d490179f3 100644
> --- a/Documentation/filesystems/sysfs.rst
> +++ b/Documentation/filesystems/sysfs.rst
> @@ -241,12 +241,10 @@ Other notes:
> is 4096.
>
> - show() methods should return the number of bytes printed into the
> - buffer. This is the return value of scnprintf().
> + buffer.
>
> -- show() must not use snprintf() when formatting the value to be
> - returned to user space. If you can guarantee that an overflow
> - will never happen you can use sprintf() otherwise you must use
> - scnprintf().
> +- show() should only use sysfs_emit() or sysfs_emit_at() when formatting
> + the value to be returned to user space.
>
> - store() should return the number of bytes used from the buffer. If the
> entire buffer has been used, just return the count argument.
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index eb6897ab78e7..96d0da65e088 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -15,6 +15,7 @@
> #include <linux/list.h>
> #include <linux/mutex.h>
> #include <linux/seq_file.h>
> +#include <linux/mm.h>
>
> #include "sysfs.h"
>
> @@ -707,3 +708,57 @@ int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid)
> return 0;
> }
> EXPORT_SYMBOL_GPL(sysfs_change_owner);
> +
> +/**
> + * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer.
> + * @buf: start of PAGE_SIZE buffer.
> + * @fmt: format
> + * @...: optional arguments to @format
> + *
> + *
> + * Returns number of characters written to @buf.
> + */
> +int sysfs_emit(char *buf, const char *fmt, ...)
> +{
> + va_list args;
> + int len;
> +
> + if (WARN(!buf || offset_in_page(buf),
> + "invalid sysfs_emit: buf:%p\n", buf))
> + return 0;
> +
> + va_start(args, fmt);
> + len = vscnprintf(buf, PAGE_SIZE, fmt, args);
> + va_end(args);
> +
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(sysfs_emit);
> +
> +/**
> + * sysfs_emit_at - scnprintf equivalent, aware of PAGE_SIZE buffer.
> + * @buf: start of PAGE_SIZE buffer.
> + * @at: offset in @buf to start write in bytes
> + * @at must be >= 0 && < PAGE_SIZE
> + * @fmt: format
> + * @...: optional arguments to @fmt
> + *
> + *
> + * Returns number of characters written starting at &@buf[@at].
> + */
> +int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
> +{
> + va_list args;
> + int len;
> +
> + if (WARN(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE,
> + "invalid sysfs_emit_at: buf:%p at:%d\n", buf, at))
> + return 0;
> +
> + va_start(args, fmt);
> + len = vscnprintf(buf + at, PAGE_SIZE - at, fmt, args);
> + va_end(args);
> +
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(sysfs_emit_at);
These feel sane, but I'm loath to have a ton of churn for no good
reason.
If we make all sysfs show/store functions use these calls instead of
sprintf(), it "feels" like that might address the objections that people
have had in the past where they are nervous about "bare" sprintf()
calls, right?
It also might make things easier to audit where we can see much easier
where sysfs files are doing "foolish" things by calling sysfs_emit_at()
a bunch of times they shouldn't be, and maybe automate the documentation
of sysfs files in a better way.
So I guess I'm asking for another developer to at least agree that this
feels like the right way forward here. I don't want to start down this
path, only to roll them all back as it feels like pointless churn.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-09-30 11:57 ` Greg Kroah-Hartman
@ 2020-09-30 13:58 ` Joe Perches
2020-10-02 11:25 ` Greg Kroah-Hartman
2020-10-01 4:17 ` Kees Cook
1 sibling, 1 reply; 26+ messages in thread
From: Joe Perches @ 2020-09-30 13:58 UTC (permalink / raw)
To: Greg Kroah-Hartman, Kees Cook, Rafael J . Wysocki
Cc: Denis Efremov, Julia Lawall, Alex Dewar, linux-kernel,
Jonathan Corbet, linux-doc
On Wed, 2020-09-30 at 13:57 +0200, Greg Kroah-Hartman wrote:
> Kees, and Rafael, I don't know if you saw this proposal from Joe for
> sysfs files, questions below:
https://lore.kernel.org/linux-pm/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git.joe@perches.com/T/
> So I guess I'm asking for another developer to at least agree that this
> feels like the right way forward here. I don't want to start down this
> path, only to roll them all back as it feels like pointless churn.
https://lore.kernel.org/lkml/c256eba42a564c01a8e470320475d46f@AcuMS.aculab.com/T/#mb40d265bc1dabb8bb64b0dfa29dd8eda44be056e
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-09-30 11:57 ` Greg Kroah-Hartman
2020-09-30 13:58 ` Joe Perches
@ 2020-10-01 4:17 ` Kees Cook
2020-10-01 4:22 ` Joe Perches
2020-10-01 20:50 ` Greg Kroah-Hartman
1 sibling, 2 replies; 26+ messages in thread
From: Kees Cook @ 2020-10-01 4:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> Kees, and Rafael, I don't know if you saw this proposal from Joe for
> sysfs files, questions below:
I'm a fan. I think the use of sprintf() in sysfs might have been one of
my earliest complaints about unsafe code patterns in the kernel. ;)
> > +/**
> > + * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer.
> > + * @buf: start of PAGE_SIZE buffer.
> > + * @fmt: format
> > + * @...: optional arguments to @format
> > + *
> > + *
> > + * Returns number of characters written to @buf.
> > + */
> > +int sysfs_emit(char *buf, const char *fmt, ...)
> > +{
> > + va_list args;
> > + int len;
> > +
> > + if (WARN(!buf || offset_in_page(buf),
> > + "invalid sysfs_emit: buf:%p\n", buf))
I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE
variant:
if (WARN_ONCE(!buf || offset_in_page(buf),
"invalid sysfs_emit: offset_in_page(buf):%zd\n",
buf ? offset_in_page(buf) : 0))
> > + return 0;
> > +
> > + va_start(args, fmt);
> > + len = vscnprintf(buf, PAGE_SIZE, fmt, args);
> > + va_end(args);
> > +
> > + return len;
> > +}
> > +EXPORT_SYMBOL_GPL(sysfs_emit);
> > +
> > +/**
> > + * sysfs_emit_at - scnprintf equivalent, aware of PAGE_SIZE buffer.
> > + * @buf: start of PAGE_SIZE buffer.
> > + * @at: offset in @buf to start write in bytes
> > + * @at must be >= 0 && < PAGE_SIZE
> > + * @fmt: format
> > + * @...: optional arguments to @fmt
> > + *
> > + *
> > + * Returns number of characters written starting at &@buf[@at].
> > + */
> > +int sysfs_emit_at(char *buf, int at, const char *fmt, ...)
> > +{
> > + va_list args;
> > + int len;
> > +
> > + if (WARN(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE,
> > + "invalid sysfs_emit_at: buf:%p at:%d\n", buf, at))
Same:
if (WARN_ONCE(!buf || offset_in_page(buf) || at < 0 || at >= PAGE_SIZE,
"invalid sysfs_emit_at: offset_in_page(buf):%zd at:%d\n",
buf ? offset_in_page(buf) : 0, at))
> > + return 0;
> > +
> > + va_start(args, fmt);
> > + len = vscnprintf(buf + at, PAGE_SIZE - at, fmt, args);
> > + va_end(args);
> > +
> > + return len;
> > +}
> > +EXPORT_SYMBOL_GPL(sysfs_emit_at);
>
> These feel sane, but I'm loath to have a ton of churn for no good
> reason.
I think the churn is worth it if we remove "seemingly wrong" code
patterns from the kernel. It's especially useful, IMO, for when there
are future mutations/refactorings and we don't end up with a bare
sprintf somewhere else.
> If we make all sysfs show/store functions use these calls instead of
> sprintf(), it "feels" like that might address the objections that people
> have had in the past where they are nervous about "bare" sprintf()
> calls, right?
I would think so. This is the kind of thing we did for %n in seq_file:
remove potential foot-gun API in favor of subsystem-specific safe API.
> It also might make things easier to audit where we can see much easier
> where sysfs files are doing "foolish" things by calling sysfs_emit_at()
> a bunch of times they shouldn't be, and maybe automate the documentation
> of sysfs files in a better way.
Indeed!
> So I guess I'm asking for another developer to at least agree that this
> feels like the right way forward here. I don't want to start down this
> path, only to roll them all back as it feels like pointless churn.
With the changes above, I'd Ack it. :)
--
Kees Cook
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-01 4:17 ` Kees Cook
@ 2020-10-01 4:22 ` Joe Perches
2020-10-02 22:30 ` Kees Cook
2020-10-01 20:50 ` Greg Kroah-Hartman
1 sibling, 1 reply; 26+ messages in thread
From: Joe Perches @ 2020-10-01 4:22 UTC (permalink / raw)
To: Kees Cook, Greg Kroah-Hartman
Cc: Rafael J . Wysocki, Denis Efremov, Julia Lawall, Alex Dewar,
linux-kernel, Jonathan Corbet, linux-doc
On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote:
> On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > sysfs files, questions below:
>
> I'm a fan. I think the use of sprintf() in sysfs might have been one of
> my earliest complaints about unsafe code patterns in the kernel. ;)
[]
> > > + if (WARN(!buf || offset_in_page(buf),
> > > + "invalid sysfs_emit: buf:%p\n", buf))
The dump_stack() is also going to emit pointers
so I don't see how it does anything but help
show where the buffer was. It is hashed...
> I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE
> variant:
>
> if (WARN_ONCE(!buf || offset_in_page(buf),
> "invalid sysfs_emit: offset_in_page(buf):%zd\n",
> buf ? offset_in_page(buf) : 0))
I don't think that helps as multiple defects can easily
exist. Log spam in this case isn't horrible either.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-01 4:17 ` Kees Cook
2020-10-01 4:22 ` Joe Perches
@ 2020-10-01 20:50 ` Greg Kroah-Hartman
2020-10-02 22:27 ` Kees Cook
1 sibling, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-01 20:50 UTC (permalink / raw)
To: Kees Cook
Cc: Joe Perches, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Wed, Sep 30, 2020 at 09:17:03PM -0700, Kees Cook wrote:
> On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > sysfs files, questions below:
>
> I'm a fan. I think the use of sprintf() in sysfs might have been one of
> my earliest complaints about unsafe code patterns in the kernel. ;)
Ok, great.
> > > +/**
> > > + * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer.
> > > + * @buf: start of PAGE_SIZE buffer.
> > > + * @fmt: format
> > > + * @...: optional arguments to @format
> > > + *
> > > + *
> > > + * Returns number of characters written to @buf.
> > > + */
> > > +int sysfs_emit(char *buf, const char *fmt, ...)
> > > +{
> > > + va_list args;
> > > + int len;
> > > +
> > > + if (WARN(!buf || offset_in_page(buf),
> > > + "invalid sysfs_emit: buf:%p\n", buf))
>
> I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE
> variant:
>
> if (WARN_ONCE(!buf || offset_in_page(buf),
> "invalid sysfs_emit: offset_in_page(buf):%zd\n",
> buf ? offset_in_page(buf) : 0))
As Joe points out, _ONCE doesn't work because this happens from all
sysfs files, not just one.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-09-30 13:58 ` Joe Perches
@ 2020-10-02 11:25 ` Greg Kroah-Hartman
0 siblings, 0 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-02 11:25 UTC (permalink / raw)
To: Joe Perches
Cc: Kees Cook, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Wed, Sep 30, 2020 at 06:58:53AM -0700, Joe Perches wrote:
> On Wed, 2020-09-30 at 13:57 +0200, Greg Kroah-Hartman wrote:
> > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > sysfs files, questions below:
>
> https://lore.kernel.org/linux-pm/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git.joe@perches.com/T/
>
> > So I guess I'm asking for another developer to at least agree that this
> > feels like the right way forward here. I don't want to start down this
> > path, only to roll them all back as it feels like pointless churn.
>
> https://lore.kernel.org/lkml/c256eba42a564c01a8e470320475d46f@AcuMS.aculab.com/T/#mb40d265bc1dabb8bb64b0dfa29dd8eda44be056e
>
>
>
>
All now queued up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-01 20:50 ` Greg Kroah-Hartman
@ 2020-10-02 22:27 ` Kees Cook
0 siblings, 0 replies; 26+ messages in thread
From: Kees Cook @ 2020-10-02 22:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Thu, Oct 01, 2020 at 10:50:29PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 30, 2020 at 09:17:03PM -0700, Kees Cook wrote:
> > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > > sysfs files, questions below:
> >
> > I'm a fan. I think the use of sprintf() in sysfs might have been one of
> > my earliest complaints about unsafe code patterns in the kernel. ;)
>
> Ok, great.
>
> > > > +/**
> > > > + * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer.
> > > > + * @buf: start of PAGE_SIZE buffer.
> > > > + * @fmt: format
> > > > + * @...: optional arguments to @format
> > > > + *
> > > > + *
> > > > + * Returns number of characters written to @buf.
> > > > + */
> > > > +int sysfs_emit(char *buf, const char *fmt, ...)
> > > > +{
> > > > + va_list args;
> > > > + int len;
> > > > +
> > > > + if (WARN(!buf || offset_in_page(buf),
> > > > + "invalid sysfs_emit: buf:%p\n", buf))
> >
> > I don't want the %p here, but otherwise, sure. I'd also make it a _ONCE
> > variant:
> >
> > if (WARN_ONCE(!buf || offset_in_page(buf),
> > "invalid sysfs_emit: offset_in_page(buf):%zd\n",
> > buf ? offset_in_page(buf) : 0))
>
> As Joe points out, _ONCE doesn't work because this happens from all
> sysfs files, not just one.
Sure, it's just a question if you want log spamming vs how reachable you
think something might be. I would expect this to be uncommon to
encounter, but very repeatable for whatever system DOES hit it, so doing
_ONCE means they see the report and don't get completely flooded with
it.
I'm fine either way.
--
Kees Cook
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-01 4:22 ` Joe Perches
@ 2020-10-02 22:30 ` Kees Cook
2020-10-03 13:55 ` Greg Kroah-Hartman
0 siblings, 1 reply; 26+ messages in thread
From: Kees Cook @ 2020-10-02 22:30 UTC (permalink / raw)
To: Joe Perches
Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Denis Efremov,
Julia Lawall, Alex Dewar, linux-kernel, Jonathan Corbet,
linux-doc
On Wed, Sep 30, 2020 at 09:22:19PM -0700, Joe Perches wrote:
> On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote:
> > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > > sysfs files, questions below:
> >
> > I'm a fan. I think the use of sprintf() in sysfs might have been one of
> > my earliest complaints about unsafe code patterns in the kernel. ;)
> []
> > > > + if (WARN(!buf || offset_in_page(buf),
> > > > + "invalid sysfs_emit: buf:%p\n", buf))
>
> The dump_stack() is also going to emit pointers
> so I don't see how it does anything but help
> show where the buffer was. It is hashed...
dump_stack() is going to report symbols and register contents.
I was just pointing out that %p has no value here[1]. The interesting
states are: "was it NULL?" "how offset was it?". Its actual content
won't matter.
-Kees
[1] "New uses of %p should not be added to the kernel"
https://www.kernel.org/doc/html/latest/process/deprecated.html#p-format-specifier
--
Kees Cook
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-02 22:30 ` Kees Cook
@ 2020-10-03 13:55 ` Greg Kroah-Hartman
2020-10-03 14:59 ` Joe Perches
2020-10-03 22:15 ` Kees Cook
0 siblings, 2 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-03 13:55 UTC (permalink / raw)
To: Kees Cook
Cc: Joe Perches, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Fri, Oct 02, 2020 at 03:30:30PM -0700, Kees Cook wrote:
> On Wed, Sep 30, 2020 at 09:22:19PM -0700, Joe Perches wrote:
> > On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote:
> > > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > > > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > > > sysfs files, questions below:
> > >
> > > I'm a fan. I think the use of sprintf() in sysfs might have been one of
> > > my earliest complaints about unsafe code patterns in the kernel. ;)
> > []
> > > > > + if (WARN(!buf || offset_in_page(buf),
> > > > > + "invalid sysfs_emit: buf:%p\n", buf))
> >
> > The dump_stack() is also going to emit pointers
> > so I don't see how it does anything but help
> > show where the buffer was. It is hashed...
>
> dump_stack() is going to report symbols and register contents.
>
> I was just pointing out that %p has no value here[1]. The interesting
> states are: "was it NULL?" "how offset was it?". Its actual content
> won't matter.
Ok, suggestions for a better error message are always welcome :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-03 13:55 ` Greg Kroah-Hartman
@ 2020-10-03 14:59 ` Joe Perches
2020-10-03 22:15 ` Kees Cook
1 sibling, 0 replies; 26+ messages in thread
From: Joe Perches @ 2020-10-03 14:59 UTC (permalink / raw)
To: Greg Kroah-Hartman, Kees Cook
Cc: Rafael J . Wysocki, Denis Efremov, Julia Lawall, Alex Dewar,
linux-kernel, Jonathan Corbet, linux-doc
On Sat, 2020-10-03 at 15:55 +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 02, 2020 at 03:30:30PM -0700, Kees Cook wrote:
> > On Wed, Sep 30, 2020 at 09:22:19PM -0700, Joe Perches wrote:
> > > On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote:
> > > > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > > > > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > > > > sysfs files, questions below:
> > > >
> > > > I'm a fan. I think the use of sprintf() in sysfs might have been one of
> > > > my earliest complaints about unsafe code patterns in the kernel. ;)
> > > []
> > > > > > + if (WARN(!buf || offset_in_page(buf),
> > > > > > + "invalid sysfs_emit: buf:%p\n", buf))
> > >
> > > The dump_stack() is also going to emit pointers
> > > so I don't see how it does anything but help
> > > show where the buffer was. It is hashed...
> >
> > dump_stack() is going to report symbols and register contents.
> >
> > I was just pointing out that %p has no value here[1]. The interesting
> > states are: "was it NULL?" "how offset was it?". Its actual content
> > won't matter.
>
> Ok, suggestions for a better error message are always welcome :)
For sysfs_emit, the offset_in_buf test also
effectively checks if buf not PAGE_SIZE so
it helps identify if it is being called from
a non _show function.
That's actually why %p can be somewhat valuable.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
2020-10-03 13:55 ` Greg Kroah-Hartman
2020-10-03 14:59 ` Joe Perches
@ 2020-10-03 22:15 ` Kees Cook
1 sibling, 0 replies; 26+ messages in thread
From: Kees Cook @ 2020-10-03 22:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, Rafael J . Wysocki, Denis Efremov, Julia Lawall,
Alex Dewar, linux-kernel, Jonathan Corbet, linux-doc
On Sat, Oct 03, 2020 at 03:55:51PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 02, 2020 at 03:30:30PM -0700, Kees Cook wrote:
> > On Wed, Sep 30, 2020 at 09:22:19PM -0700, Joe Perches wrote:
> > > On Wed, 2020-09-30 at 21:17 -0700, Kees Cook wrote:
> > > > On Wed, Sep 30, 2020 at 01:57:40PM +0200, Greg Kroah-Hartman wrote:
> > > > > Kees, and Rafael, I don't know if you saw this proposal from Joe for
> > > > > sysfs files, questions below:
> > > >
> > > > I'm a fan. I think the use of sprintf() in sysfs might have been one of
> > > > my earliest complaints about unsafe code patterns in the kernel. ;)
> > > []
> > > > > > + if (WARN(!buf || offset_in_page(buf),
> > > > > > + "invalid sysfs_emit: buf:%p\n", buf))
> > >
> > > The dump_stack() is also going to emit pointers
> > > so I don't see how it does anything but help
> > > show where the buffer was. It is hashed...
> >
> > dump_stack() is going to report symbols and register contents.
> >
> > I was just pointing out that %p has no value here[1]. The interesting
> > states are: "was it NULL?" "how offset was it?". Its actual content
> > won't matter.
>
> Ok, suggestions for a better error message are always welcome :)
... I did. :P
https://lore.kernel.org/lkml/202009302108.18B05CA38@keescook/
But it doesn't need to hold up the series; %p is hashed, so I don't
care. It's just that the mandate from Linus was to not add new %p uses.
:P
--
Kees Cook
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-10-03 22:15 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-16 20:40 [PATCH V3 0/8] sysfs: drivers core: Add and use sysfs_emit and sysfs_emit_at Joe Perches
2020-09-16 20:40 ` [PATCH V3 1/8] sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output Joe Perches
2020-09-30 11:57 ` Greg Kroah-Hartman
2020-09-30 13:58 ` Joe Perches
2020-10-02 11:25 ` Greg Kroah-Hartman
2020-10-01 4:17 ` Kees Cook
2020-10-01 4:22 ` Joe Perches
2020-10-02 22:30 ` Kees Cook
2020-10-03 13:55 ` Greg Kroah-Hartman
2020-10-03 14:59 ` Joe Perches
2020-10-03 22:15 ` Kees Cook
2020-10-01 20:50 ` Greg Kroah-Hartman
2020-10-02 22:27 ` Kees Cook
2020-09-16 20:40 ` [PATCH V3 2/8] drivers core: Use sysfs_emit and sysfs_emit_at for show(device *...) functions Joe Perches
2020-09-16 20:40 ` [PATCH V3 3/8] drivers core: Remove strcat uses around sysfs_emit and neaten Joe Perches
2020-09-16 20:40 ` [PATCH V3 4/8] drivers core: Reindent a couple uses around sysfs_emit Joe Perches
2020-09-16 20:40 ` [PATCH V3 5/8] drivers core: Miscellaneous changes for sysfs_emit Joe Perches
2020-09-16 20:40 ` [PATCH V3 6/8] mm: and drivers core: Convert hugetlb_report_node_meminfo to sysfs_emit Joe Perches
2020-09-18 20:29 ` Mike Kravetz
2020-09-19 6:22 ` Greg Kroah-Hartman
2020-09-19 16:51 ` Joe Perches
2020-09-25 18:32 ` Joe Perches
2020-09-29 0:53 ` Joe Perches
2020-09-29 4:17 ` Greg Kroah-Hartman
2020-09-16 20:40 ` [PATCH V3 7/8] drivers core: Use sysfs_emit for shared_cpu_map_show and shared_cpu_list_show Joe Perches
2020-09-16 20:40 ` [PATCH V3 8/8] drivers core: node: Use a more typical macro definition style for ACCESS_ATTR Joe Perches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).