Re: [patch 02/46] genirq/irqdesc: Switch to lock guards

From: Jiri Slaby
Date: Fri Mar 14 2025 - 06:57:34 EST


On 13. 03. 25, 16:59, Thomas Gleixner wrote:
Replace all lock/unlock pairs with lock guards and simplify the code flow.

No functional change.

Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
---
kernel/irq/irqdesc.c | 136 +++++++++++++++------------------------------------
1 file changed, 42 insertions(+), 94 deletions(-)

--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -266,104 +266,68 @@ static ssize_t per_cpu_count_show(struct
}
IRQ_ATTR_RO(per_cpu_count);
-static ssize_t chip_name_show(struct kobject *kobj,
- struct kobj_attribute *attr, char *buf)
+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;
-
- 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);
- }
- raw_spin_unlock_irq(&desc->lock);
- return ret;
+ guard(raw_spinlock_irq)(&desc->lock);
+ if (desc->irq_data.chip && desc->irq_data.chip->name)
+ return scnprintf(buf, PAGE_SIZE, "%s\n", desc->irq_data.chip->name);
+ return 0;

FWIW I consider this ^^^ ...

}
IRQ_ATTR_RO(chip_name);
-static ssize_t hwirq_show(struct kobject *kobj,
- struct kobj_attribute *attr, char *buf)
+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;
-
- raw_spin_lock_irq(&desc->lock);
- if (desc->irq_data.domain)
- ret = sprintf(buf, "%lu\n", desc->irq_data.hwirq);
- raw_spin_unlock_irq(&desc->lock);
- return ret;
+ guard(raw_spinlock_irq)(&desc->lock);
+ return desc->irq_data.domain ? sprintf(buf, "%lu\n", desc->irq_data.hwirq) : 0;

... more readable than this ^^^. (Ie. I would preserve the 'if'. Even though here is only a simple condition.)

}
IRQ_ATTR_RO(hwirq);

-static ssize_t name_show(struct kobject *kobj,
- struct kobj_attribute *attr, char *buf)
+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;
- raw_spin_lock_irq(&desc->lock);
- if (desc->name)
- ret = scnprintf(buf, PAGE_SIZE, "%s\n", desc->name);
- raw_spin_unlock_irq(&desc->lock);
-
- return ret;
+ guard(raw_spinlock_irq)(&desc->lock);

You do guard() ...

+ return desc->name ? scnprintf(buf, PAGE_SIZE, "%s\n", desc->name) : 0;
}
IRQ_ATTR_RO(name);
-static ssize_t actions_show(struct kobject *kobj,
- struct kobj_attribute *attr, char *buf)
+static ssize_t actions_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
{
struct irq_desc *desc = container_of(kobj, struct irq_desc, kobj);
struct irqaction *action;
ssize_t ret = 0;
char *p = "";
- raw_spin_lock_irq(&desc->lock);
- for_each_action_of_desc(desc, action) {
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s",
- p, action->name);
- p = ",";
+ scoped_guard (raw_spinlock_irq, &desc->lock) {

... but scoped_guard<space>(). Unlike in internals.h where you do scoped_guard(). Any reason for that?

+ for_each_action_of_desc(desc, action) {
+ ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s", p, action->name);
+ p = ",";
+ }
}
- raw_spin_unlock_irq(&desc->lock);
-
- if (ret)
- ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n");
- return ret;
+ return ret ? ret + scnprintf(buf + ret, PAGE_SIZE - ret, "\n") : 0;
}
IRQ_ATTR_RO(actions);
...
@@ -573,12 +532,12 @@ static int alloc_descs(unsigned int star
return -ENOMEM;
}
-static int irq_expand_nr_irqs(unsigned int nr)
+static bool irq_expand_nr_irqs(unsigned int nr)
{
if (nr > MAX_SPARSE_IRQS)
- return -ENOMEM;
+ return false;
nr_irqs = nr;
- return 0;
+ return true;

This is sort of unrelated to $SUBJ ^^^. In any case:

@@ -681,14 +638,13 @@ static inline int alloc_descs(unsigned i
static int irq_expand_nr_irqs(unsigned int nr)

s/int/bool/ here too.

{
- return -ENOMEM;
+ return false;
}

thanks,
--
js
suse labs