linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] perf/x86: Use sysfs_emit() in show() callback function
@ 2023-02-26 17:29 Deepak R Varma
  2023-02-26 17:30 ` [PATCH v3 1/3] perf/x86/core: " Deepak R Varma
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Deepak R Varma @ 2023-02-26 17:29 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-perf-users, linux-kernel
  Cc: Saurabh Singh Sengar, Praveen Kumar, Deepak R Varma

The patch set proposes to replace use of sprintf family function usage for
formatting data to be returned to the user space using show callback functions.

The modified files have the same maintainer list though they correspond to
different drivers. Since its the same, I clubbed these into a patch set. If this
is not correct, I can send in individual driver specific patches.

Changes in v3:
   - For patch 1, an erroneous change proposed was removed.
     Feedback provided by Peter Zijlstra <peterz@infradead.org>
   - No changes for Patch 2 and 3 of the series.

Changes in v2:
   1. Update the patch log message to provide details on the problem associated
      with current implementation and how the proposal is a better solution.
      Feedback provided by Peter Zijlstra <peterz@infradead.org>


Deepak R Varma (3):
  perf/x86/core: Use sysfs_emit() in show() callback function
  perf/x86/intel/pt: Use sysfs_emit() in show() callback function
  perf/x86/intel: Use sysfs_emit() in show() callback function

 arch/x86/events/core.c       | 4 ++--
 arch/x86/events/intel/core.c | 6 +++---
 arch/x86/events/intel/pt.c   | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.34.1




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/3] perf/x86/core: Use sysfs_emit() in show() callback function
  2023-02-26 17:29 [PATCH v3 0/3] perf/x86: Use sysfs_emit() in show() callback function Deepak R Varma
@ 2023-02-26 17:30 ` Deepak R Varma
  2023-02-26 17:30 ` [PATCH v3 2/3] perf/x86/intel/pt: " Deepak R Varma
  2023-02-26 17:31 ` [PATCH v3 3/3] perf/x86/intel: " Deepak R Varma
  2 siblings, 0 replies; 4+ messages in thread
From: Deepak R Varma @ 2023-02-26 17:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-perf-users, linux-kernel
  Cc: Saurabh Singh Sengar, Praveen Kumar, Deepak R Varma

As per C99 standard, snprintf() returns the number of bytes that would
be encoded in the destination buffer when it is sufficiently large. This
return value may be different from what the caller is expecting and hence
may lead to potential errors in the program.
Kernel release 2.6.2 introduced scnprintf() & vscnprintf() which precisely
return the actual bytes encoded into the destination buffer.

For the sysfs attribute show() callback functions, which returns the number
of bytes to the user space, a more recent recommendation is to use
sysfs_emit() or sysfs_emit_at() instead of sprintf() family of functions.
This is recorded in the Documentation/filesystems/sysfs.rst Kernel
documentation file.

Issue identified using the coccinelle device_attr_show.cocci script.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
Changes in v3:
   - Remove incorrect/erroneous change proposed.
     Feedback provided by Peter Zijlstra <peterz@infradead.org>

Changes in v2:
   - Revise patch log message to include details on the potential issues with
     current implementation and how the proposal is a better solution.
     Feedback provided by Peter Zijlstra <peterz@infradead.org>


 arch/x86/events/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index d096b04bf80e..87a7f0cd77fd 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2544,7 +2544,7 @@ static ssize_t get_attr_rdpmc(struct device *cdev,
 			      struct device_attribute *attr,
 			      char *buf)
 {
-	return snprintf(buf, 40, "%d\n", x86_pmu.attr_rdpmc);
+	return sysfs_emit(buf, "%d\n", x86_pmu.attr_rdpmc);
 }
 
 static ssize_t set_attr_rdpmc(struct device *cdev,
@@ -2602,7 +2602,7 @@ static ssize_t max_precise_show(struct device *cdev,
 				  struct device_attribute *attr,
 				  char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", x86_pmu_max_precise());
+	return sysfs_emit(buf, "%d\n", x86_pmu_max_precise());
 }
 
 static DEVICE_ATTR_RO(max_precise);
-- 
2.34.1




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/3] perf/x86/intel/pt: Use sysfs_emit() in show() callback function
  2023-02-26 17:29 [PATCH v3 0/3] perf/x86: Use sysfs_emit() in show() callback function Deepak R Varma
  2023-02-26 17:30 ` [PATCH v3 1/3] perf/x86/core: " Deepak R Varma
@ 2023-02-26 17:30 ` Deepak R Varma
  2023-02-26 17:31 ` [PATCH v3 3/3] perf/x86/intel: " Deepak R Varma
  2 siblings, 0 replies; 4+ messages in thread
From: Deepak R Varma @ 2023-02-26 17:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-perf-users, linux-kernel
  Cc: Saurabh Singh Sengar, Praveen Kumar, Deepak R Varma

As per C99 standard, snprintf() returns the number of bytes that would
be encoded in the destination buffer when it is sufficiently large. This
return value may be different from what the caller is expecting and hence
may lead to potential errors in the program.
Kernel release 2.6.2 introduced scnprintf() & vscnprintf() which precisely
return the actual bytes encoded into the destination buffer.

For the sysfs attribute show() callback functions, which returns the number
of bytes to the user space, a more recent recommendation is to use
sysfs_emit() or sysfs_emit_at() instead of sprintf() family of functions.
This is recorded in the Documentation/filesystems/sysfs.rst Kernel
documentation file.

Issue identified using the coccinelle device_attr_show.cocci script.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
Changes in v3:
   - None.

Changes in v2:
   - Revise patch log message to include details on the potential issues with
     current implementation and how the proposal is a better solution.
     Feedback provided by Peter Zijlstra <peterz@infradead.org>

 arch/x86/events/intel/pt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 42a55794004a..d9e6d771b458 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -96,7 +96,7 @@ static ssize_t pt_cap_show(struct device *cdev,
 		container_of(attr, struct dev_ext_attribute, attr);
 	enum pt_capabilities cap = (long)ea->var;
 
-	return snprintf(buf, PAGE_SIZE, "%x\n", intel_pt_validate_hw_cap(cap));
+	return sysfs_emit(buf, "%x\n", intel_pt_validate_hw_cap(cap));
 }
 
 static struct attribute_group pt_cap_group __ro_after_init = {
-- 
2.34.1




^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 3/3] perf/x86/intel: Use sysfs_emit() in show() callback function
  2023-02-26 17:29 [PATCH v3 0/3] perf/x86: Use sysfs_emit() in show() callback function Deepak R Varma
  2023-02-26 17:30 ` [PATCH v3 1/3] perf/x86/core: " Deepak R Varma
  2023-02-26 17:30 ` [PATCH v3 2/3] perf/x86/intel/pt: " Deepak R Varma
@ 2023-02-26 17:31 ` Deepak R Varma
  2 siblings, 0 replies; 4+ messages in thread
From: Deepak R Varma @ 2023-02-26 17:31 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, linux-perf-users, linux-kernel
  Cc: Saurabh Singh Sengar, Praveen Kumar, Deepak R Varma

As per C99 standard, snprintf() returns the number of bytes that would
be encoded in the destination buffer when it is sufficiently large. This
return value may be different from what the caller is expecting and hence
may lead to potential errors in the program.
Kernel release 2.6.2 introduced scnprintf() & vscnprintf() which precisely
return the actual bytes encoded into the destination buffer.

For the sysfs attribute show() callback functions, which returns the number
of bytes to the user space, a more recent recommendation is to use
sysfs_emit() or sysfs_emit_at() instead of sprintf() family of functions.
This is recorded in the Documentation/filesystems/sysfs.rst Kernel
documentation file.

Issue identified using the coccinelle device_attr_show.cocci script.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---
Changes in v3:
   - None.

Changes in v2:
   - Revise patch log message to include details on the potential issues with
     current implementation and how the proposal is a better solution.
     Feedback provided by Peter Zijlstra <peterz@infradead.org>

 arch/x86/events/intel/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 9fce2d1247a7..75ab89ff3a0f 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -5387,7 +5387,7 @@ static ssize_t show_sysctl_tfa(struct device *cdev,
 			      struct device_attribute *attr,
 			      char *buf)
 {
-	return snprintf(buf, 40, "%d\n", allow_tsx_force_abort);
+	return sysfs_emit(buf, "%d\n", allow_tsx_force_abort);
 }
 
 static ssize_t set_sysctl_tfa(struct device *cdev,
@@ -5421,7 +5421,7 @@ static ssize_t branches_show(struct device *cdev,
 			     struct device_attribute *attr,
 			     char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%d\n", x86_pmu.lbr_nr);
+	return sysfs_emit(buf, "%d\n", x86_pmu.lbr_nr);
 }
 
 static DEVICE_ATTR_RO(branches);
@@ -5437,7 +5437,7 @@ static ssize_t pmu_name_show(struct device *cdev,
 			     struct device_attribute *attr,
 			     char *buf)
 {
-	return snprintf(buf, PAGE_SIZE, "%s\n", pmu_name_str);
+	return sysfs_emit(buf, "%s\n", pmu_name_str);
 }
 
 static DEVICE_ATTR_RO(pmu_name);
-- 
2.34.1




^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-26 17:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-26 17:29 [PATCH v3 0/3] perf/x86: Use sysfs_emit() in show() callback function Deepak R Varma
2023-02-26 17:30 ` [PATCH v3 1/3] perf/x86/core: " Deepak R Varma
2023-02-26 17:30 ` [PATCH v3 2/3] perf/x86/intel/pt: " Deepak R Varma
2023-02-26 17:31 ` [PATCH v3 3/3] perf/x86/intel: " Deepak R Varma

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