[PATCH] genirq/irqdesc: Use sysfs_emit in the <foo>_show functions

[PATCH] genirq/irqdesc: Use sysfs_emit in the <foo>_show functions

[Joe Perches]

Convert the nominally unbounded sprintf output to use sysfs_emit and the
scnprintf uses to sysfs_emit_at.

Miscellanea:

o sysfs_emit: Use the more common int len not ssize_t ret
o hwirq_show: Add a minimum newline output when irq_data.domain is not set
  and remove an unnecessary cast of an unsigned long to int ("%d" -> "%lu)
o name_show: Add a minimum newline output when desc->name is not set

trivially reduces object size (x86-64 defconfig, gcc 10.3)

$ size kernel/irq/irqdesc.o*
   text	   data	    bss	    dec	    hex	filename
   5809	    576	   1608	   7993	   1f39	kernel/irq/irqdesc.o.new
   5896	    576	   1608	   8080	   1f90	kernel/irq/irqdesc.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
 kernel/irq/irqdesc.c | 65 ++++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index fadb937660202..8d47b8667c989 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -147,19 +147,20 @@ static ssize_t per_cpu_count_show(struct kobject *kobj,
 				  struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
-	char *p = "";
+	int len = 0;
+	const char *p = "";
 	int cpu;
 
 	for_each_possible_cpu(cpu) {
 		unsigned int c = irq_desc_kstat_cpu(desc, cpu);
 
-		ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%u", p, c);
+		len += sysfs_emit_at(buf, len, "%s%u", p, c);
 		p = ",";
 	}
 
-	ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
-	return ret;
+	len += sysfs_emit_at(buf, len, "\n");
+
+	return len;
 }
 IRQ_ATTR_RO(per_cpu_count);
 
@@ -167,16 +168,15 @@ static ssize_t chip_name_show(struct kobject *kobj,
 			      struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
+	int len = 0;
 
 	raw_spin_lock_irq(&desc->lock);
-	if (desc->irq_data.chip && desc->irq_data.chip->name) {
-		ret = scnprintf(buf, PAGE_SIZE, "%s\n",
-				desc->irq_data.chip->name);
-	}
+	len = sysfs_emit(buf, "%s\n",
+			 desc->irq_data.chip && desc->irq_data.chip->name ?
+			 desc->irq_data.chip->name : "");
 	raw_spin_unlock_irq(&desc->lock);
 
-	return ret;
+	return len;
 }
 IRQ_ATTR_RO(chip_name);
 
@@ -184,14 +184,16 @@ static ssize_t hwirq_show(struct kobject *kobj,
 			  struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
+	int len = 0;
 
 	raw_spin_lock_irq(&desc->lock);
 	if (desc->irq_data.domain)
-		ret = sprintf(buf, "%d\n", (int)desc->irq_data.hwirq);
+		len = sysfs_emit(buf, "%lu\n", desc->irq_data.hwirq);
+	else
+		len = sysfs_emit(buf, "\n");
 	raw_spin_unlock_irq(&desc->lock);
 
-	return ret;
+	return len;
 }
 IRQ_ATTR_RO(hwirq);
 
@@ -199,14 +201,14 @@ static ssize_t type_show(struct kobject *kobj,
 			 struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
+	int len = 0;
 
 	raw_spin_lock_irq(&desc->lock);
-	ret = sprintf(buf, "%s\n",
-		      irqd_is_level_type(&desc->irq_data) ? "level" : "edge");
+	len = sysfs_emit(buf, "%s\n",
+			 irqd_is_level_type(&desc->irq_data) ? "level" : "edge");
 	raw_spin_unlock_irq(&desc->lock);
 
-	return ret;
+	return len;
 
 }
 IRQ_ATTR_RO(type);
@@ -215,14 +217,14 @@ static ssize_t wakeup_show(struct kobject *kobj,
 			   struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
+	int len = 0;
 
 	raw_spin_lock_irq(&desc->lock);
-	ret = sprintf(buf, "%s\n",
-		      irqd_is_wakeup_set(&desc->irq_data) ? "enabled" : "disabled");
+	len = sysfs_emit(buf, "%s\n",
+			 irqd_is_wakeup_set(&desc->irq_data) ? "enabled" : "disabled");
 	raw_spin_unlock_irq(&desc->lock);
 
-	return ret;
+	return len;
 
 }
 IRQ_ATTR_RO(wakeup);
@@ -231,14 +233,13 @@ static ssize_t name_show(struct kobject *kobj,
 			 struct kobj_attribute *attr, char *buf)
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
-	ssize_t ret = 0;
+	int len = 0;
 
 	raw_spin_lock_irq(&desc->lock);
-	if (desc->name)
-		ret = scnprintf(buf, PAGE_SIZE, "%s\n", desc->name);
+	len = sysfs_emit(buf, "%s\n", desc->name ?: "");
 	raw_spin_unlock_irq(&desc->lock);
 
-	return ret;
+	return len;
 }
 IRQ_ATTR_RO(name);
 
@@ -247,21 +248,19 @@ static ssize_t actions_show(struct kobject *kobj,
 {
 	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
 	struct irqaction *action;
-	ssize_t ret = 0;
-	char *p = "";
+	int len = 0;
+	const char *p = "";
 
 	raw_spin_lock_irq(&desc->lock);
 	for (action = desc->action; action != NULL; action = action->next) {
-		ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s",
-				 p, action->name);
+		len += sysfs_emit_at(buf, len, "%s%s", p, action->name);
 		p = ",";
 	}
 	raw_spin_unlock_irq(&desc->lock);
 
-	if (ret)
-		ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
+	len += sysfs_emit_at(buf, len, "\n");
 
-	return ret;
+	return len;
 }
 IRQ_ATTR_RO(actions);
 

Re: [PATCH] genirq/irqdesc: Use sysfs_emit in the <foo>_show functions

[Thomas Gleixner]

Joe!

On Tue, Aug 10 2021 at 10:55, Joe Perches wrote:
> Convert the nominally unbounded sprintf output to use sysfs_emit and the
> scnprintf uses to sysfs_emit_at.

Please use function_name() not function_name as that makes it clear what
this is about.

> Miscellanea:

The point is?

> o sysfs_emit: Use the more common int len not ssize_t ret

You're not changing sysfs_emit(). You change the variables in the irq
functions.

> o hwirq_show: Add a minimum newline output when irq_data.domain is not set
>   and remove an unnecessary cast of an unsigned long to int ("%d" ->
>   "%lu)

That's a separate change and has nothing to do with $subject

> o name_show: Add a minimum newline output when desc->name is not set

Ditto.

> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index fadb937660202..8d47b8667c989 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -147,19 +147,20 @@ static ssize_t per_cpu_count_show(struct kobject *kobj,
>  				  struct kobj_attribute *attr, char *buf)
>  {
>  	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
> -	ssize_t ret = 0;
> -	char *p = "";
> +	int len = 0;
> +	const char *p = "";
>  	int cpu;

Please keep reverse fir tree ordering and stick the two int variables
into one line, i.e. 

  	struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
	const char *p = "";
	int cpu, len = 0;

Please rework against

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip irq/core

Thanks,

        tglx