* [PATCH 1/9] irq/irqdesc: put the lock at the exact place in irq_sysfs_init()
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 14:05 ` [PATCH 2/9] irq/irqdesc: change the name of delete_irq_desc() to irq_delete_desc() Pingfan Liu
` (7 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel; +Cc: Pingfan Liu, Thomas Gleixner
sparse_irq_lock is used to protect irq_desc, and furthermore
irq_sysfs_init() is called only once, so put irq_lock_sparse() at the
exact place, where the race condition may start.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
To: linux-kernel@vger.kernel.org
---
kernel/irq/irqdesc.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 939d21cd55c3..8d0982233277 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -312,15 +312,14 @@ static int __init irq_sysfs_init(void)
struct irq_desc *desc;
int irq;
- /* Prevent concurrent irq alloc/free */
- irq_lock_sparse();
-
irq_kobj_base = kobject_create_and_add("irq", kernel_kobj);
if (!irq_kobj_base) {
- irq_unlock_sparse();
return -ENOMEM;
}
+ /* Prevent concurrent irq alloc/free */
+ irq_lock_sparse();
+
/* Add the already allocated interrupts */
for_each_irq_desc(irq, desc)
irq_sysfs_add(irq, desc);
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 2/9] irq/irqdesc: change the name of delete_irq_desc() to irq_delete_desc()
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
2022-04-20 14:05 ` [PATCH 1/9] irq/irqdesc: put the lock at the exact place in irq_sysfs_init() Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 14:05 ` [PATCH 3/9] irq/manage: remove some unreferenced code Pingfan Liu
` (6 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel; +Cc: Pingfan Liu, Thomas Gleixner
Make the naming convention consistent with irq_insert_desc(),
irq_to_desc().
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
To: linux-kernel@vger.kernel.org
---
kernel/irq/irqdesc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 8d0982233277..9feedaa08430 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -355,7 +355,7 @@ struct irq_desc *irq_to_desc(unsigned int irq)
EXPORT_SYMBOL_GPL(irq_to_desc);
#endif
-static void delete_irq_desc(unsigned int irq)
+static void irq_delete_desc(unsigned int irq)
{
radix_tree_delete(&irq_desc_tree, irq);
}
@@ -453,7 +453,7 @@ static void free_desc(unsigned int irq)
* irq_sysfs_init() as well.
*/
irq_sysfs_del(desc);
- delete_irq_desc(irq);
+ irq_delete_desc(irq);
/*
* We free the descriptor, masks and stat fields via RCU. That
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 3/9] irq/manage: remove some unreferenced code
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
2022-04-20 14:05 ` [PATCH 1/9] irq/irqdesc: put the lock at the exact place in irq_sysfs_init() Pingfan Liu
2022-04-20 14:05 ` [PATCH 2/9] irq/irqdesc: change the name of delete_irq_desc() to irq_delete_desc() Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 20:56 ` Miguel Ojeda
2022-04-20 14:05 ` [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt() Pingfan Liu
` (5 subsequent siblings)
8 siblings, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel
Cc: Pingfan Liu, Miguel Ojeda, Thomas Gleixner, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Marc Zyngier, Florian Fainelli,
Bartosz Golaszewski, Cédric Le Goater, llvm
Neither remove_percpu_irq() nor for_each_irq_nr() has any users, so they
can be removed.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Tom Rix <trix@redhat.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: llvm@lists.linux.dev
To: linux-kernel@vger.kernel.org
---
.clang-format | 1 -
include/linux/irq.h | 1 -
include/linux/irqnr.h | 3 ---
kernel/irq/manage.c | 15 ---------------
4 files changed, 20 deletions(-)
diff --git a/.clang-format b/.clang-format
index fa959436bcfd..fb161bc8ca1a 100644
--- a/.clang-format
+++ b/.clang-format
@@ -198,7 +198,6 @@ ForEachMacros:
- 'for_each_if'
- 'for_each_iommu'
- 'for_each_ip_tunnel_rcu'
- - 'for_each_irq_nr'
- 'for_each_link_codecs'
- 'for_each_link_cpus'
- 'for_each_link_platforms'
diff --git a/include/linux/irq.h b/include/linux/irq.h
index f92788ccdba2..ec50007decfc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -603,7 +603,6 @@ enum {
struct irqaction;
extern int setup_percpu_irq(unsigned int irq, struct irqaction *new);
-extern void remove_percpu_irq(unsigned int irq, struct irqaction *act);
#ifdef CONFIG_DEPRECATED_IRQ_CPU_ONOFFLINE
extern void irq_cpu_online(void);
diff --git a/include/linux/irqnr.h b/include/linux/irqnr.h
index 3496baa0b07f..082fe0856e87 100644
--- a/include/linux/irqnr.h
+++ b/include/linux/irqnr.h
@@ -28,7 +28,4 @@ unsigned int irq_get_next_irq(unsigned int offset);
for (irq = irq_get_next_irq(0); irq < nr_irqs; \
irq = irq_get_next_irq(irq + 1))
-#define for_each_irq_nr(irq) \
- for (irq = 0; irq < nr_irqs; irq++)
-
#endif
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index c03f71d5ec10..b7c86be68b58 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2466,21 +2466,6 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_
return NULL;
}
-/**
- * remove_percpu_irq - free a per-cpu interrupt
- * @irq: Interrupt line to free
- * @act: irqaction for the interrupt
- *
- * Used to remove interrupts statically setup by the early boot process.
- */
-void remove_percpu_irq(unsigned int irq, struct irqaction *act)
-{
- struct irq_desc *desc = irq_to_desc(irq);
-
- if (desc && irq_settings_is_per_cpu_devid(desc))
- __free_percpu_irq(irq, act->percpu_dev_id);
-}
-
/**
* free_percpu_irq - free an interrupt allocated with request_percpu_irq
* @irq: Interrupt line to free
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 3/9] irq/manage: remove some unreferenced code
2022-04-20 14:05 ` [PATCH 3/9] irq/manage: remove some unreferenced code Pingfan Liu
@ 2022-04-20 20:56 ` Miguel Ojeda
0 siblings, 0 replies; 26+ messages in thread
From: Miguel Ojeda @ 2022-04-20 20:56 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-kernel, Miguel Ojeda, Thomas Gleixner, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Marc Zyngier, Florian Fainelli,
Bartosz Golaszewski, Cédric Le Goater, llvm
On Wed, Apr 20, 2022 at 4:05 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> Neither remove_percpu_irq() nor for_each_irq_nr() has any users, so they
> can be removed.
For `.clang-format`:
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (2 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 3/9] irq/manage: remove some unreferenced code Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 18:16 ` Heiko Carstens
2022-04-22 10:02 ` [PATCHv2] " Pingfan Liu
2022-04-20 14:05 ` [PATCH 5/9] x86/irq: place for_each_active_irq() in rcu read section Pingfan Liu
` (4 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-s390
Cc: Pingfan Liu, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
irq_desc can be accessed safely in RCU read section as demonstrated by
kstat_irqs_usr(). And raw_spin_lock_irqsave() context can provide a rcu
read section, which can be utilized to get rid of irq_lock_sparse().
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org
To: linux-s390@vger.kernel.org
---
arch/s390/kernel/irq.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 3033f616e256..6302dc7874cf 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -205,12 +205,13 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
unsigned long flags;
int cpu;
- irq_lock_sparse();
+ raw_spin_lock_irqsave(&desc->lock, flags);
desc = irq_to_desc(irq);
- if (!desc)
- goto out;
+ if (!desc) {
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+ return;
+ }
- raw_spin_lock_irqsave(&desc->lock, flags);
seq_printf(p, "%3d: ", irq);
for_each_online_cpu(cpu)
seq_printf(p, "%10u ", irq_desc_kstat_cpu(desc, cpu));
@@ -223,8 +224,6 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
seq_putc(p, '\n');
raw_spin_unlock_irqrestore(&desc->lock, flags);
-out:
- irq_unlock_sparse();
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-20 14:05 ` [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt() Pingfan Liu
@ 2022-04-20 18:16 ` Heiko Carstens
2022-04-21 3:36 ` Pingfan Liu
2022-04-22 10:02 ` [PATCHv2] " Pingfan Liu
1 sibling, 1 reply; 26+ messages in thread
From: Heiko Carstens @ 2022-04-20 18:16 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
On Wed, Apr 20, 2022 at 10:05:16PM +0800, Pingfan Liu wrote:
> irq_desc can be accessed safely in RCU read section as demonstrated by
> kstat_irqs_usr(). And raw_spin_lock_irqsave() context can provide a rcu
> read section, which can be utilized to get rid of irq_lock_sparse().
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> To: linux-s390@vger.kernel.org
> ---
> arch/s390/kernel/irq.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> index 3033f616e256..6302dc7874cf 100644
> --- a/arch/s390/kernel/irq.c
> +++ b/arch/s390/kernel/irq.c
> @@ -205,12 +205,13 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> unsigned long flags;
> int cpu;
>
> - irq_lock_sparse();
> + raw_spin_lock_irqsave(&desc->lock, flags);
> desc = irq_to_desc(irq);
How is this supposed to work? desc get's initialized after its random
stack value has been used as a pointer to lock something...
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-20 18:16 ` Heiko Carstens
@ 2022-04-21 3:36 ` Pingfan Liu
2022-04-21 11:42 ` Heiko Carstens
0 siblings, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-21 3:36 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
On Wed, Apr 20, 2022 at 08:16:37PM +0200, Heiko Carstens wrote:
> On Wed, Apr 20, 2022 at 10:05:16PM +0800, Pingfan Liu wrote:
> > irq_desc can be accessed safely in RCU read section as demonstrated by
> > kstat_irqs_usr(). And raw_spin_lock_irqsave() context can provide a rcu
> > read section, which can be utilized to get rid of irq_lock_sparse().
> >
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > Cc: Heiko Carstens <hca@linux.ibm.com>
> > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> > Cc: Sven Schnelle <svens@linux.ibm.com>
> > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Cc: linux-kernel@vger.kernel.org
> > To: linux-s390@vger.kernel.org
> > ---
> > arch/s390/kernel/irq.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> > index 3033f616e256..6302dc7874cf 100644
> > --- a/arch/s390/kernel/irq.c
> > +++ b/arch/s390/kernel/irq.c
> > @@ -205,12 +205,13 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> > unsigned long flags;
> > int cpu;
> >
> > - irq_lock_sparse();
> > + raw_spin_lock_irqsave(&desc->lock, flags);
> > desc = irq_to_desc(irq);
>
> How is this supposed to work? desc get's initialized after its random
> stack value has been used as a pointer to lock something...
Oops. You are right. What about using rcu_read_lock() directly?
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 3033f616e256..45393919fe61 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -205,7 +205,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
unsigned long flags;
int cpu;
- irq_lock_sparse();
+ rcu_read_lock();
desc = irq_to_desc(irq);
if (!desc)
goto out;
@@ -224,7 +224,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
seq_putc(p, '\n');
raw_spin_unlock_irqrestore(&desc->lock, flags);
out:
- irq_unlock_sparse();
+ rcu_read_unlock();
}
/*
Thanks,
Pingfan
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-21 3:36 ` Pingfan Liu
@ 2022-04-21 11:42 ` Heiko Carstens
2022-04-22 9:56 ` Pingfan Liu
0 siblings, 1 reply; 26+ messages in thread
From: Heiko Carstens @ 2022-04-21 11:42 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
On Thu, Apr 21, 2022 at 11:36:17AM +0800, Pingfan Liu wrote:
> Oops. You are right. What about using rcu_read_lock() directly?
>
>
> diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> index 3033f616e256..45393919fe61 100644
> --- a/arch/s390/kernel/irq.c
> +++ b/arch/s390/kernel/irq.c
> @@ -205,7 +205,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> unsigned long flags;
> int cpu;
>
> - irq_lock_sparse();
> + rcu_read_lock();
> desc = irq_to_desc(irq);
> if (!desc)
> goto out;
> @@ -224,7 +224,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> seq_putc(p, '\n');
> raw_spin_unlock_irqrestore(&desc->lock, flags);
> out:
> - irq_unlock_sparse();
> + rcu_read_unlock();
That looks like it should work. Please resend and also add a reference
to commit 74bdf7815dfb ("genirq: Speedup show_interrupts()") which
explains why this works.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-21 11:42 ` Heiko Carstens
@ 2022-04-22 9:56 ` Pingfan Liu
0 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-22 9:56 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
On Thu, Apr 21, 2022 at 01:42:59PM +0200, Heiko Carstens wrote:
> On Thu, Apr 21, 2022 at 11:36:17AM +0800, Pingfan Liu wrote:
> > Oops. You are right. What about using rcu_read_lock() directly?
> >
> >
> > diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
> > index 3033f616e256..45393919fe61 100644
> > --- a/arch/s390/kernel/irq.c
> > +++ b/arch/s390/kernel/irq.c
> > @@ -205,7 +205,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> > unsigned long flags;
> > int cpu;
> >
> > - irq_lock_sparse();
> > + rcu_read_lock();
> > desc = irq_to_desc(irq);
> > if (!desc)
> > goto out;
> > @@ -224,7 +224,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
> > seq_putc(p, '\n');
> > raw_spin_unlock_irqrestore(&desc->lock, flags);
> > out:
> > - irq_unlock_sparse();
> > + rcu_read_unlock();
>
> That looks like it should work. Please resend and also add a reference
> to commit 74bdf7815dfb ("genirq: Speedup show_interrupts()") which
> explains why this works.
Thanks for your review. I will follow up with V2.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-20 14:05 ` [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt() Pingfan Liu
2022-04-20 18:16 ` Heiko Carstens
@ 2022-04-22 10:02 ` Pingfan Liu
2022-04-25 11:39 ` Heiko Carstens
1 sibling, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-22 10:02 UTC (permalink / raw)
To: linux-s390
Cc: Pingfan Liu, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
As demonstrated by commit 74bdf7815dfb ("genirq: Speedup show_interrupts()"),
irq_desc can be accessed safely in RCU read section.
Hence here resorting to rcu read lock to get rid of irq_lock_sparse().
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org
To: linux-s390@vger.kernel.org
---
arch/s390/kernel/irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/kernel/irq.c b/arch/s390/kernel/irq.c
index 3033f616e256..45393919fe61 100644
--- a/arch/s390/kernel/irq.c
+++ b/arch/s390/kernel/irq.c
@@ -205,7 +205,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
unsigned long flags;
int cpu;
- irq_lock_sparse();
+ rcu_read_lock();
desc = irq_to_desc(irq);
if (!desc)
goto out;
@@ -224,7 +224,7 @@ static void show_msi_interrupt(struct seq_file *p, int irq)
seq_putc(p, '\n');
raw_spin_unlock_irqrestore(&desc->lock, flags);
out:
- irq_unlock_sparse();
+ rcu_read_unlock();
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCHv2] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt()
2022-04-22 10:02 ` [PATCHv2] " Pingfan Liu
@ 2022-04-25 11:39 ` Heiko Carstens
0 siblings, 0 replies; 26+ messages in thread
From: Heiko Carstens @ 2022-04-25 11:39 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-s390, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sebastian Andrzej Siewior,
linux-kernel
On Fri, Apr 22, 2022 at 06:02:12PM +0800, Pingfan Liu wrote:
> As demonstrated by commit 74bdf7815dfb ("genirq: Speedup show_interrupts()"),
> irq_desc can be accessed safely in RCU read section.
>
> Hence here resorting to rcu read lock to get rid of irq_lock_sparse().
>
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> To: linux-s390@vger.kernel.org
>
> ---
> arch/s390/kernel/irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied, thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/9] x86/irq: place for_each_active_irq() in rcu read section
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (3 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 4/9] s390/irq: utilize RCU instead of irq_lock_sparse() in show_msi_interrupt() Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 14:05 ` [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release Pingfan Liu
` (3 subsequent siblings)
8 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: x86
Cc: Pingfan Liu, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Maciej W. Rozycki, Marc Zyngier,
linux-kernel
Since there are access to irq_desc, and no preemption is provided at the
involved, it requires rcu read lock to protect irq_desc.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Marc Zyngier <maz@kernel.org>
Cc: linux-kernel@vger.kernel.org
To: x86@kernel.org
---
arch/x86/kernel/apic/io_apic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index c1bb384935b0..4bb16edcbe4d 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1333,6 +1333,7 @@ void __init print_IO_APICs(void)
print_IO_APIC(ioapic_idx);
printk(KERN_DEBUG "IRQ to pin mappings:\n");
+ rcu_read_lock();
for_each_active_irq(irq) {
struct irq_pin_list *entry;
struct irq_chip *chip;
@@ -1352,6 +1353,7 @@ void __init print_IO_APICs(void)
pr_cont("-> %d:%d", entry->apic, entry->pin);
pr_cont("\n");
}
+ rcu_read_unlock();
printk(KERN_INFO ".................................... done.\n");
}
@@ -2009,6 +2011,7 @@ static inline void init_IO_APIC_traps(void)
struct irq_cfg *cfg;
unsigned int irq;
+ /* The early boot stage is free of irq_desc release */
for_each_active_irq(irq) {
cfg = irq_cfg(irq);
if (IO_APIC_IRQ(irq) && cfg && !cfg->vector) {
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (4 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 5/9] x86/irq: place for_each_active_irq() in rcu read section Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 16:23 ` Rafael J. Wysocki
2022-04-27 6:03 ` [PATCHv2] genirq/PM: Make " Pingfan Liu
2022-04-20 14:05 ` [PATCH 7/9] irq: remove needless lock in takedown_cpu() Pingfan Liu
` (2 subsequent siblings)
8 siblings, 2 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel; +Cc: Pingfan Liu, Thomas Gleixner, Rafael J. Wysocki
The invloved context is no a RCU read section. Furthermore there may be
more than one task at this point. Hence it demands a measure to prevent
irq_desc from freeing. Use irq_lock_sparse to serve the protection
purpose.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
To: linux-kernel@vger.kernel.org
---
kernel/irq/pm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index ca71123a6130..4b67a4c7de3c 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -133,6 +133,7 @@ void suspend_device_irqs(void)
struct irq_desc *desc;
int irq;
+ irq_lock_sparse();
for_each_irq_desc(irq, desc) {
unsigned long flags;
bool sync;
@@ -146,6 +147,7 @@ void suspend_device_irqs(void)
if (sync)
synchronize_irq(irq);
}
+ irq_unlock_sparse();
}
EXPORT_SYMBOL_GPL(suspend_device_irqs);
@@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
struct irq_desc *desc;
int irq;
+ /* The early resume stage is free of irq_desc release */
for_each_irq_desc(irq, desc) {
unsigned long flags;
bool is_early = desc->action &&
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release
2022-04-20 14:05 ` [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release Pingfan Liu
@ 2022-04-20 16:23 ` Rafael J. Wysocki
2022-04-21 3:31 ` Pingfan Liu
2022-04-27 6:03 ` [PATCHv2] genirq/PM: Make " Pingfan Liu
1 sibling, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2022-04-20 16:23 UTC (permalink / raw)
To: Pingfan Liu; +Cc: Linux Kernel Mailing List, Thomas Gleixner, Rafael J. Wysocki
On Wed, Apr 20, 2022 at 4:06 PM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> The invloved context is no a RCU read section. Furthermore there may be
> more than one task at this point. Hence it demands a measure to prevent
> irq_desc from freeing. Use irq_lock_sparse to serve the protection
> purpose.
Can you please describe an example scenario in which the added locking
will prevent a failure from occurring?
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> To: linux-kernel@vger.kernel.org
> ---
> kernel/irq/pm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> index ca71123a6130..4b67a4c7de3c 100644
> --- a/kernel/irq/pm.c
> +++ b/kernel/irq/pm.c
> @@ -133,6 +133,7 @@ void suspend_device_irqs(void)
> struct irq_desc *desc;
> int irq;
>
> + irq_lock_sparse();
> for_each_irq_desc(irq, desc) {
> unsigned long flags;
> bool sync;
> @@ -146,6 +147,7 @@ void suspend_device_irqs(void)
> if (sync)
> synchronize_irq(irq);
> }
> + irq_unlock_sparse();
> }
> EXPORT_SYMBOL_GPL(suspend_device_irqs);
>
> @@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
> struct irq_desc *desc;
> int irq;
>
> + /* The early resume stage is free of irq_desc release */
> for_each_irq_desc(irq, desc) {
> unsigned long flags;
> bool is_early = desc->action &&
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release
2022-04-20 16:23 ` Rafael J. Wysocki
@ 2022-04-21 3:31 ` Pingfan Liu
2022-04-21 10:57 ` Rafael J. Wysocki
0 siblings, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-21 3:31 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux Kernel Mailing List, Thomas Gleixner
On Wed, Apr 20, 2022 at 06:23:48PM +0200, Rafael J. Wysocki wrote:
> On Wed, Apr 20, 2022 at 4:06 PM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > The invloved context is no a RCU read section. Furthermore there may be
> > more than one task at this point. Hence it demands a measure to prevent
> > irq_desc from freeing. Use irq_lock_sparse to serve the protection
> > purpose.
>
> Can you please describe an example scenario in which the added locking
> will prevent a failure from occurring?
>
Sorry to forget mentioning that this is based on the code analysis.
Suppose the following scenario:
Two threads invloved
threadA "hibernate" runs suspend_device_irqs()
threadB "rcu_cpu_kthread" runs rcu_core()->rcu_do_batch(), which releases
object, let's say irq_desc
Zoom in:
threadA threadB
for_each_irq_desc(irq, desc) {
get irq_descA which is under freeing
--->preempted by rcu_core()->rcu_do_batch() which releases irq_descA
raw_spin_lock_irqsave(&desc->lock, flags);
//Oops
And since in the involved code piece, threadA runs in a preemptible
context, and there may be more than one thread at this stage. So the
preempted can happen.
Thanks,
Pingfan
> > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > To: linux-kernel@vger.kernel.org
> > ---
> > kernel/irq/pm.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> > index ca71123a6130..4b67a4c7de3c 100644
> > --- a/kernel/irq/pm.c
> > +++ b/kernel/irq/pm.c
> > @@ -133,6 +133,7 @@ void suspend_device_irqs(void)
> > struct irq_desc *desc;
> > int irq;
> >
> > + irq_lock_sparse();
> > for_each_irq_desc(irq, desc) {
> > unsigned long flags;
> > bool sync;
> > @@ -146,6 +147,7 @@ void suspend_device_irqs(void)
> > if (sync)
> > synchronize_irq(irq);
> > }
> > + irq_unlock_sparse();
> > }
> > EXPORT_SYMBOL_GPL(suspend_device_irqs);
> >
> > @@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
> > struct irq_desc *desc;
> > int irq;
> >
> > + /* The early resume stage is free of irq_desc release */
> > for_each_irq_desc(irq, desc) {
> > unsigned long flags;
> > bool is_early = desc->action &&
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release
2022-04-21 3:31 ` Pingfan Liu
@ 2022-04-21 10:57 ` Rafael J. Wysocki
2022-04-22 10:43 ` Pingfan Liu
0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2022-04-21 10:57 UTC (permalink / raw)
To: Pingfan Liu; +Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Thomas Gleixner
On Thu, Apr 21, 2022 at 5:31 AM Pingfan Liu <kernelfans@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 06:23:48PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Apr 20, 2022 at 4:06 PM Pingfan Liu <kernelfans@gmail.com> wrote:
> > >
> > > The invloved context is no a RCU read section. Furthermore there may be
> > > more than one task at this point. Hence it demands a measure to prevent
> > > irq_desc from freeing. Use irq_lock_sparse to serve the protection
> > > purpose.
> >
> > Can you please describe an example scenario in which the added locking
> > will prevent a failure from occurring?
> >
>
> Sorry to forget mentioning that this is based on the code analysis.
>
> Suppose the following scenario:
> Two threads invloved
> threadA "hibernate" runs suspend_device_irqs()
> threadB "rcu_cpu_kthread" runs rcu_core()->rcu_do_batch(), which releases
> object, let's say irq_desc
>
> Zoom in:
> threadA threadB
> for_each_irq_desc(irq, desc) {
> get irq_descA which is under freeing
> --->preempted by rcu_core()->rcu_do_batch() which releases irq_descA
> raw_spin_lock_irqsave(&desc->lock, flags);
> //Oops
>
> And since in the involved code piece, threadA runs in a preemptible
> context, and there may be more than one thread at this stage. So the
> preempted can happen.
Well, I'm still not sure that this can ever trigger in practice, but I
guess the locking can be added for extra safety.
Anyway, the above information should go into the changelog IMO.
That said ->
> > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > To: linux-kernel@vger.kernel.org
> > > ---
> > > kernel/irq/pm.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> > > index ca71123a6130..4b67a4c7de3c 100644
> > > --- a/kernel/irq/pm.c
> > > +++ b/kernel/irq/pm.c
> > > @@ -133,6 +133,7 @@ void suspend_device_irqs(void)
> > > struct irq_desc *desc;
> > > int irq;
> > >
> > > + irq_lock_sparse();
> > > for_each_irq_desc(irq, desc) {
> > > unsigned long flags;
> > > bool sync;
> > > @@ -146,6 +147,7 @@ void suspend_device_irqs(void)
> > > if (sync)
> > > synchronize_irq(irq);
-> is it entirely safe to call synchronize_irq() under irq_lock_sparse?
> > > }
> > > + irq_unlock_sparse();
> > > }
> > > EXPORT_SYMBOL_GPL(suspend_device_irqs);
> > >
> > > @@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
> > > struct irq_desc *desc;
> > > int irq;
> > >
> > > + /* The early resume stage is free of irq_desc release */
> > > for_each_irq_desc(irq, desc) {
> > > unsigned long flags;
> > > bool is_early = desc->action &&
> > > --
> > > 2.31.1
> > >
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release
2022-04-21 10:57 ` Rafael J. Wysocki
@ 2022-04-22 10:43 ` Pingfan Liu
0 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-22 10:43 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux Kernel Mailing List, Thomas Gleixner
On Thu, Apr 21, 2022 at 12:57:28PM +0200, Rafael J. Wysocki wrote:
> On Thu, Apr 21, 2022 at 5:31 AM Pingfan Liu <kernelfans@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 06:23:48PM +0200, Rafael J. Wysocki wrote:
> > > On Wed, Apr 20, 2022 at 4:06 PM Pingfan Liu <kernelfans@gmail.com> wrote:
> > > >
> > > > The invloved context is no a RCU read section. Furthermore there may be
> > > > more than one task at this point. Hence it demands a measure to prevent
> > > > irq_desc from freeing. Use irq_lock_sparse to serve the protection
> > > > purpose.
> > >
> > > Can you please describe an example scenario in which the added locking
> > > will prevent a failure from occurring?
> > >
> >
> > Sorry to forget mentioning that this is based on the code analysis.
> >
> > Suppose the following scenario:
> > Two threads invloved
> > threadA "hibernate" runs suspend_device_irqs()
> > threadB "rcu_cpu_kthread" runs rcu_core()->rcu_do_batch(), which releases
> > object, let's say irq_desc
> >
> > Zoom in:
> > threadA threadB
> > for_each_irq_desc(irq, desc) {
> > get irq_descA which is under freeing
> > --->preempted by rcu_core()->rcu_do_batch() which releases irq_descA
> > raw_spin_lock_irqsave(&desc->lock, flags);
> > //Oops
> >
> > And since in the involved code piece, threadA runs in a preemptible
> > context, and there may be more than one thread at this stage. So the
> > preempted can happen.
>
> Well, I'm still not sure that this can ever trigger in practice, but I
Yes, I also think it hardly happen. I had gone through all
accesses to irq_desc in kernel, and just want to make anything
completely obey the rule.
> guess the locking can be added for extra safety.
>
> Anyway, the above information should go into the changelog IMO.
>
OK, I will update it in V2.
> That said ->
>
> > > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> > > > To: linux-kernel@vger.kernel.org
> > > > ---
> > > > kernel/irq/pm.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> > > > index ca71123a6130..4b67a4c7de3c 100644
> > > > --- a/kernel/irq/pm.c
> > > > +++ b/kernel/irq/pm.c
> > > > @@ -133,6 +133,7 @@ void suspend_device_irqs(void)
> > > > struct irq_desc *desc;
> > > > int irq;
> > > >
> > > > + irq_lock_sparse();
> > > > for_each_irq_desc(irq, desc) {
> > > > unsigned long flags;
> > > > bool sync;
> > > > @@ -146,6 +147,7 @@ void suspend_device_irqs(void)
> > > > if (sync)
> > > > synchronize_irq(irq);
>
> -> is it entirely safe to call synchronize_irq() under irq_lock_sparse?
synchronize_irq - wait for pending IRQ handlers (on other CPUs). It
only holds irq_desc->lock and has no connections with irq sparse tree or
bitmap. I can not see any deadlock issue or miss something?
Thanks for your time.
Regards,
Pingfan
>
> > > > }
> > > > + irq_unlock_sparse();
> > > > }
> > > > EXPORT_SYMBOL_GPL(suspend_device_irqs);
> > > >
> > > > @@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
> > > > struct irq_desc *desc;
> > > > int irq;
> > > >
> > > > + /* The early resume stage is free of irq_desc release */
> > > > for_each_irq_desc(irq, desc) {
> > > > unsigned long flags;
> > > > bool is_early = desc->action &&
> > > > --
> > > > 2.31.1
> > > >
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2] genirq/PM: Make for_each_irq_desc() safe of irq_desc release
2022-04-20 14:05 ` [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release Pingfan Liu
2022-04-20 16:23 ` Rafael J. Wysocki
@ 2022-04-27 6:03 ` Pingfan Liu
1 sibling, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-27 6:03 UTC (permalink / raw)
To: linux-kernel; +Cc: Pingfan Liu, Thomas Gleixner, Rafael J. Wysocki
First, this is a suspicion of the code, not a really encountered bug.
*** The scenario ***
Two threads involved
threadA "hibernate" runs suspend_device_irqs()
threadB "rcu_cpu_kthread" runs rcu_core()->rcu_do_batch(), which releases
object, let's say irq_desc
Zoom in:
threadA threadB
for_each_irq_desc(irq, desc) {
get irq_descA which is under freeing
--->preempted by rcu_core()->rcu_do_batch() which releases irq_descA
raw_spin_lock_irqsave(&desc->lock, flags);
//Oops
And since in the involved code piece, suspend_device_irqs() runs in a
preemptible context, and there may be more than one thread at this
point. So the preemption can happen.
*** The fix ***
Since there is a blockable synchronize_irq() inside the code piece,
resorting to irq_lock_sparse() to protect the irq_desc from
disappearing.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
To: linux-kernel@vger.kernel.org
---
v1 -> v2: improve commit log
kernel/irq/pm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index ca71123a6130..4b67a4c7de3c 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -133,6 +133,7 @@ void suspend_device_irqs(void)
struct irq_desc *desc;
int irq;
+ irq_lock_sparse();
for_each_irq_desc(irq, desc) {
unsigned long flags;
bool sync;
@@ -146,6 +147,7 @@ void suspend_device_irqs(void)
if (sync)
synchronize_irq(irq);
}
+ irq_unlock_sparse();
}
EXPORT_SYMBOL_GPL(suspend_device_irqs);
@@ -186,6 +188,7 @@ static void resume_irqs(bool want_early)
struct irq_desc *desc;
int irq;
+ /* The early resume stage is free of irq_desc release */
for_each_irq_desc(irq, desc) {
unsigned long flags;
bool is_early = desc->action &&
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 7/9] irq: remove needless lock in takedown_cpu()
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (5 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 6/9] pm/irq: make for_each_irq_desc() safe of irq_desc release Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-21 16:11 ` Thomas Gleixner
2022-04-20 14:05 ` [PATCH 8/9] irq: make irq_lock_sparse() independent of CONFIG_SPARSE_IRQ Pingfan Liu
2022-04-20 14:05 ` [PATCH 9/9] irq/irqdesc: rename sparse_irq_lock to bitmap_lock Pingfan Liu
8 siblings, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel
Cc: Pingfan Liu, Thomas Gleixner, Valentin Schneider, Peter Zijlstra,
Frederic Weisbecker, Baokun Li, Mark Rutland, Steven Price,
Jason A. Donenfeld, Yuan ZhaoXiong, YueHaibing, Randy Dunlap
Any allocation or release of irq_desc should fall into the interface of
__irq_alloc_descs() or irq_free_descs(). And both of them hold the mutex
sparse_irq_lock. This preemptability contradicts the preempt-disabled
context when dispatching fn in cpu_stopper_thread(). So allocation or
free of irq_desc can not be demanded in cpu_stopper_thread().
On the other hand, for the safety of access to irq_desc, rcu still keeps
watching the dying cpu until the last minute rcu_report_dead(), so each
cpu_stop_fn_t can safely access irq_desc.
As a result, in takedown_cpu() irq_lock_sparse()/irq_unlock_sparse() can
be safely removed.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Baokun Li <libaokun1@huawei.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Steven Price <steven.price@arm.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: Yuan ZhaoXiong <yuanzhaoxiong@baidu.com>
Cc: YueHaibing <yuehaibing@huawei.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
To: linux-kernel@vger.kernel.org
---
kernel/cpu.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index d0a9aa0b42e8..94a6b512c26d 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1033,18 +1033,16 @@ static int takedown_cpu(unsigned int cpu)
kthread_park(st->thread);
/*
- * Prevent irq alloc/free while the dying cpu reorganizes the
- * interrupt affinities.
+ * RCU keeps watching 'cpu' until do_idle()->rcu_report_dead().
+ * And cpu_stopper's fn is dispatched with preemption disabled.
+ * So it can not occur to release a irq_desc.
*/
- irq_lock_sparse();
/*
* So now all preempt/rcu users must observe !cpu_active().
*/
err = stop_machine_cpuslocked(take_cpu_down, NULL, cpumask_of(cpu));
if (err) {
- /* CPU refused to die */
- irq_unlock_sparse();
/* Unpark the hotplug thread so we can rollback there */
kthread_unpark(st->thread);
return err;
@@ -1061,9 +1059,6 @@ static int takedown_cpu(unsigned int cpu)
wait_for_ap_thread(st, false);
BUG_ON(st->state != CPUHP_AP_IDLE_DEAD);
- /* Interrupts are moved away from the dying cpu, reenable alloc/free */
- irq_unlock_sparse();
-
hotplug_cpu__broadcast_tick_pull(cpu);
/* This actually kills the CPU. */
__cpu_die(cpu);
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()
2022-04-20 14:05 ` [PATCH 7/9] irq: remove needless lock in takedown_cpu() Pingfan Liu
@ 2022-04-21 16:11 ` Thomas Gleixner
2022-04-25 2:57 ` Pingfan Liu
0 siblings, 1 reply; 26+ messages in thread
From: Thomas Gleixner @ 2022-04-21 16:11 UTC (permalink / raw)
To: Pingfan Liu, linux-kernel
Cc: Pingfan Liu, Valentin Schneider, Peter Zijlstra,
Frederic Weisbecker, Baokun Li, Mark Rutland, Steven Price,
Jason A. Donenfeld, Yuan ZhaoXiong, YueHaibing, Randy Dunlap
On Wed, Apr 20 2022 at 22:05, Pingfan Liu wrote:
First of all, the subject prefix for the core interrupt subsystem is
'genirq' and the sentence after the colon starts with an uppercase
letter. See:
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index d0a9aa0b42e8..94a6b512c26d 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1033,18 +1033,16 @@ static int takedown_cpu(unsigned int cpu)
> kthread_park(st->thread);
>
> /*
> - * Prevent irq alloc/free while the dying cpu reorganizes the
> - * interrupt affinities.
> + * RCU keeps watching 'cpu' until do_idle()->rcu_report_dead().
> + * And cpu_stopper's fn is dispatched with preemption disabled.
> + * So it can not occur to release a irq_desc.
> */
> - irq_lock_sparse();
Not everything is about RCU here. You really need to look at all moving
parts:
irq_migrate_all_off_this_cpu() relies on the allocated_irqs bitmap and
the sparse tree to be in consistent state, which is only guaranteed when
the sparse lock is held.
I'm not sure what you are trying to solve here. Not taking sparse_irq_lock
here is not gaining anything.
Thanks,
tglx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()
2022-04-21 16:11 ` Thomas Gleixner
@ 2022-04-25 2:57 ` Pingfan Liu
2022-04-25 9:43 ` Thomas Gleixner
0 siblings, 1 reply; 26+ messages in thread
From: Pingfan Liu @ 2022-04-25 2:57 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Valentin Schneider, Peter Zijlstra,
Frederic Weisbecker, Baokun Li, Mark Rutland, Steven Price,
Jason A. Donenfeld, Yuan ZhaoXiong, YueHaibing, Randy Dunlap
On Thu, Apr 21, 2022 at 06:11:56PM +0200, Thomas Gleixner wrote:
> On Wed, Apr 20 2022 at 22:05, Pingfan Liu wrote:
>
> First of all, the subject prefix for the core interrupt subsystem is
> 'genirq' and the sentence after the colon starts with an uppercase
> letter. See:
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html
>
> > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > index d0a9aa0b42e8..94a6b512c26d 100644
> > --- a/kernel/cpu.c
> > +++ b/kernel/cpu.c
> > @@ -1033,18 +1033,16 @@ static int takedown_cpu(unsigned int cpu)
> > kthread_park(st->thread);
> >
> > /*
> > - * Prevent irq alloc/free while the dying cpu reorganizes the
> > - * interrupt affinities.
> > + * RCU keeps watching 'cpu' until do_idle()->rcu_report_dead().
> > + * And cpu_stopper's fn is dispatched with preemption disabled.
> > + * So it can not occur to release a irq_desc.
> > */
> > - irq_lock_sparse();
>
> Not everything is about RCU here. You really need to look at all moving
> parts:
>
> irq_migrate_all_off_this_cpu() relies on the allocated_irqs bitmap and
> the sparse tree to be in consistent state, which is only guaranteed when
> the sparse lock is held.
>
For the irq which transfer from active to inactive(disappearing) after
fetching, desc->lock can serve the sync purpose. In this case,
irq_lock_sparse() is not needed. For a emergeing irq, I am not sure
about it.
> I'm not sure what you are trying to solve here. Not taking sparse_irq_lock
> here is not gaining anything.
>
It was a big lock preventing my original series to make kexec-reboot parallel on
arm64/riscv platform. But my new series takes a different way. And this
big lock is not a problem any longer.
Thanks for your time.
Regards,
Pingfan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()
2022-04-25 2:57 ` Pingfan Liu
@ 2022-04-25 9:43 ` Thomas Gleixner
2022-04-27 6:01 ` Pingfan Liu
0 siblings, 1 reply; 26+ messages in thread
From: Thomas Gleixner @ 2022-04-25 9:43 UTC (permalink / raw)
To: Pingfan Liu
Cc: linux-kernel, Valentin Schneider, Peter Zijlstra,
Frederic Weisbecker, Baokun Li, Mark Rutland, Steven Price,
Jason A. Donenfeld, Yuan ZhaoXiong, YueHaibing, Randy Dunlap
On Mon, Apr 25 2022 at 10:57, Pingfan Liu wrote:
> On Thu, Apr 21, 2022 at 06:11:56PM +0200, Thomas Gleixner wrote:
>> > - irq_lock_sparse();
>>
>> Not everything is about RCU here. You really need to look at all moving
>> parts:
>>
>> irq_migrate_all_off_this_cpu() relies on the allocated_irqs bitmap and
>> the sparse tree to be in consistent state, which is only guaranteed when
>> the sparse lock is held.
>>
>
> For the irq which transfer from active to inactive(disappearing) after
> fetching, desc->lock can serve the sync purpose. In this case,
> irq_lock_sparse() is not needed. For a emergeing irq, I am not sure
> about it.
No, it's required for the free case. The alloc case is
uninteresting. Care to look into the code?
irq_free_descs()
lock(sparse);
free_descs();
bitmap_clear(allocated_irqs, from, cnt);
unlock_sparse);
As free_descs() sets the sparse tree entry to NULL, up to the point
where bitmap_clear() finishes the state is inconsistent.
Now look at irq_migrate_all_off_this_cpu() and figure out what happens
when stop_machine() hits into the inconsistent state.
This can be fixed, but not by making mysterious claims about RCU and
desc->lock.
Thanks,
tglx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 7/9] irq: remove needless lock in takedown_cpu()
2022-04-25 9:43 ` Thomas Gleixner
@ 2022-04-27 6:01 ` Pingfan Liu
0 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-27 6:01 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, Valentin Schneider, Peter Zijlstra,
Frederic Weisbecker, Baokun Li, Mark Rutland, Steven Price,
Jason A. Donenfeld, Yuan ZhaoXiong, YueHaibing, Randy Dunlap
On Mon, Apr 25, 2022 at 11:43:03AM +0200, Thomas Gleixner wrote:
> On Mon, Apr 25 2022 at 10:57, Pingfan Liu wrote:
> > On Thu, Apr 21, 2022 at 06:11:56PM +0200, Thomas Gleixner wrote:
> >> > - irq_lock_sparse();
> >>
> >> Not everything is about RCU here. You really need to look at all moving
> >> parts:
> >>
> >> irq_migrate_all_off_this_cpu() relies on the allocated_irqs bitmap and
> >> the sparse tree to be in consistent state, which is only guaranteed when
> >> the sparse lock is held.
> >>
> >
> > For the irq which transfer from active to inactive(disappearing) after
> > fetching, desc->lock can serve the sync purpose. In this case,
> > irq_lock_sparse() is not needed. For a emergeing irq, I am not sure
> > about it.
>
> No, it's required for the free case. The alloc case is
> uninteresting. Care to look into the code?
>
Yes, it is a good exercise. Thanks for the enlightenment.
> irq_free_descs()
> lock(sparse);
> free_descs();
> bitmap_clear(allocated_irqs, from, cnt);
> unlock_sparse);
>
> As free_descs() sets the sparse tree entry to NULL, up to the point
> where bitmap_clear() finishes the state is inconsistent.
>
> Now look at irq_migrate_all_off_this_cpu() and figure out what happens
> when stop_machine() hits into the inconsistent state.
>
So the following code should fix the inconsistence between bitmap and
sparse tree.
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 1ed2b1739363..cd0d180f082d 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -161,6 +161,8 @@ void irq_migrate_all_off_this_cpu(void)
bool affinity_broken;
desc = irq_to_desc(irq);
+ if (!desc)
+ continue;
raw_spin_lock(&desc->lock);
affinity_broken = migrate_one_irq(desc);
raw_spin_unlock(&desc->lock);
> This can be fixed, but not by making mysterious claims about RCU and
> desc->lock.
>
But I still think that desc->lock is critical to the consistence of the
irq _affinity_ if removing sparse lock in takedown_cpu().
For the free case, after applying the above patch, it should work.
void irq_migrate_all_off_this_cpu(void)
{
for_each_active_irq(irq) {
desc = irq_to_desc(irq);
if (!desc)
continue;
---> if breaking
in by free, then
migrate_one_irq()
will skip it
since the irq is
not activated any
long
raw_spin_lock(&desc->lock);
affinity_broken = migrate_one_irq(desc);
raw_spin_unlock(&desc->lock);
...
}
}
But for the alloc case, it could be a problem.
void irq_migrate_all_off_this_cpu(void)
{
for_each_active_irq(irq) {
desc = irq_to_desc(irq);
if (!desc)
continue;
raw_spin_lock(&desc->lock);
affinity_broken = migrate_one_irq(desc);
raw_spin_unlock(&desc->lock);
...
---> any new irq will
not be detected. But alloc_descs(start, cnt, node, affinity)
still associate the
irq with this cpu.
There is _no_
opportunity to clear
out this cpu from
desc->irq_common_data.affinity.
This is the affinity
inconsistent problem.
}
Thanks,
Pingfan
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 8/9] irq: make irq_lock_sparse() independent of CONFIG_SPARSE_IRQ
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (6 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 7/9] irq: remove needless lock in takedown_cpu() Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
2022-04-20 14:05 ` [PATCH 9/9] irq/irqdesc: rename sparse_irq_lock to bitmap_lock Pingfan Liu
8 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel
Cc: Pingfan Liu, Thomas Gleixner, Marc Zyngier, Wolfram Sang,
Sebastian Andrzej Siewior, Mark Rutland
Even in the case of non-sparse irq, the callsite of
for_each_active_irq() in irq_debugfs_init() still cares about the sync
of allocated_irqs bitmap. Otherwise irq_debugfs_init() may show some
disappeared irq if the irq is disactived by other driver in parallel.
As there are only a few callsites of irq_lock_sparse() in the cold path,
which means the slight performance drops can be ignored. Hence moving
irq_lock_sparse() out of CONFIG_SPARSE_IRQ to protect both irq_desc and
allocated_irqs bitmap, instead of making irq_lock_sparse() dependent on
GENERIC_IRQ_DEBUGFS.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
To: linux-kernel@vger.kernel.org
---
include/linux/irqdesc.h | 6 ++----
kernel/irq/irqdesc.c | 20 ++++++++++----------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index a77584593f7d..6c01231fec00 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -104,12 +104,10 @@ struct irq_desc {
const char *name;
} ____cacheline_internodealigned_in_smp;
-#ifdef CONFIG_SPARSE_IRQ
extern void irq_lock_sparse(void);
extern void irq_unlock_sparse(void);
-#else
-static inline void irq_lock_sparse(void) { }
-static inline void irq_unlock_sparse(void) { }
+
+#ifndef CONFIG_SPARSE_IRQ
extern struct irq_desc irq_desc[NR_IRQS];
#endif
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 9feedaa08430..a5cefd7c9ef7 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -375,16 +375,6 @@ static void free_masks(struct irq_desc *desc)
static inline void free_masks(struct irq_desc *desc) { }
#endif
-void irq_lock_sparse(void)
-{
- mutex_lock(&sparse_irq_lock);
-}
-
-void irq_unlock_sparse(void)
-{
- mutex_unlock(&sparse_irq_lock);
-}
-
static struct irq_desc *alloc_desc(int irq, int node, unsigned int flags,
const struct cpumask *affinity,
struct module *owner)
@@ -721,6 +711,16 @@ int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
}
#endif
+void irq_lock_sparse(void)
+{
+ mutex_lock(&sparse_irq_lock);
+}
+
+void irq_unlock_sparse(void)
+{
+ mutex_unlock(&sparse_irq_lock);
+}
+
/* Dynamic interrupt handling */
/**
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 9/9] irq/irqdesc: rename sparse_irq_lock to bitmap_lock
2022-04-20 14:05 [PATCH 0/9] trival fix or improvement about irq_desc access Pingfan Liu
` (7 preceding siblings ...)
2022-04-20 14:05 ` [PATCH 8/9] irq: make irq_lock_sparse() independent of CONFIG_SPARSE_IRQ Pingfan Liu
@ 2022-04-20 14:05 ` Pingfan Liu
8 siblings, 0 replies; 26+ messages in thread
From: Pingfan Liu @ 2022-04-20 14:05 UTC (permalink / raw)
To: linux-kernel
Cc: Pingfan Liu, Thomas Gleixner, Marc Zyngier, Mark Rutland,
Oleksandr Natalenko, Sebastian Andrzej Siewior, Peter Zijlstra,
Valentin Schneider, Qais Yousef, Yuan ZhaoXiong, Randy Dunlap,
Dongli Zhang, YueHaibing, Steven Price
Now that sparse_irq_lock serves for both sparse and non-sparse purpose,
it is better to rename it as bitmap_lock. Corresponding, rename
irq_lock_sparse() as irq_lock_bitmap().
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Pingfan Liu <kernelfans@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oleksandr Natalenko <oleksandr@natalenko.name>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Qais Yousef <qais.yousef@arm.com>
Cc: Yuan ZhaoXiong <yuanzhaoxiong@baidu.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Dongli Zhang <dongli.zhang@oracle.com>
Cc: YueHaibing <yuehaibing@huawei.com>
Cc: Steven Price <steven.price@arm.com>
To: linux-kernel@vger.kernel.org
---
include/linux/irqdesc.h | 4 ++--
kernel/cpu.c | 4 ++--
kernel/irq/cpuhotplug.c | 4 ++--
kernel/irq/debugfs.c | 4 ++--
kernel/irq/irqdesc.c | 26 +++++++++++++-------------
kernel/irq/pm.c | 4 ++--
6 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 6c01231fec00..8301653fc2b2 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -104,8 +104,8 @@ struct irq_desc {
const char *name;
} ____cacheline_internodealigned_in_smp;
-extern void irq_lock_sparse(void);
-extern void irq_unlock_sparse(void);
+extern void irq_lock_bitmap(void);
+extern void irq_unlock_bitmap(void);
#ifndef CONFIG_SPARSE_IRQ
extern struct irq_desc irq_desc[NR_IRQS];
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 94a6b512c26d..8874f0242893 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -601,11 +601,11 @@ static int bringup_cpu(unsigned int cpu)
* setup the vector space for the cpu which comes online.
* Prevent irq alloc/free across the bringup.
*/
- irq_lock_sparse();
+ irq_lock_bitmap();
/* Arch-specific enabling code. */
ret = __cpu_up(cpu, idle);
- irq_unlock_sparse();
+ irq_unlock_bitmap();
if (ret)
return ret;
return bringup_wait_for_ap(cpu);
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 1ed2b1739363..84547b5781be 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -220,14 +220,14 @@ int irq_affinity_online_cpu(unsigned int cpu)
struct irq_desc *desc;
unsigned int irq;
- irq_lock_sparse();
+ irq_lock_bitmap();
for_each_active_irq(irq) {
desc = irq_to_desc(irq);
raw_spin_lock_irq(&desc->lock);
irq_restore_affinity_of_irq(desc, cpu);
raw_spin_unlock_irq(&desc->lock);
}
- irq_unlock_sparse();
+ irq_unlock_bitmap();
return 0;
}
diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index 2b43f5f5033d..af4bd2ca2372 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -251,10 +251,10 @@ static int __init irq_debugfs_init(void)
irq_dir = debugfs_create_dir("irqs", root_dir);
- irq_lock_sparse();
+ irq_lock_bitmap();
for_each_active_irq(irq)
irq_add_debugfs_entry(irq, irq_to_desc(irq));
- irq_unlock_sparse();
+ irq_unlock_bitmap();
return 0;
}
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index a5cefd7c9ef7..acd80cb8a511 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -130,7 +130,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
int nr_irqs = NR_IRQS;
EXPORT_SYMBOL_GPL(nr_irqs);
-static DEFINE_MUTEX(sparse_irq_lock);
+static DEFINE_MUTEX(bitmap_lock);
static DECLARE_BITMAP(allocated_irqs, IRQ_BITMAP_BITS);
#ifdef CONFIG_SPARSE_IRQ
@@ -318,12 +318,12 @@ static int __init irq_sysfs_init(void)
}
/* Prevent concurrent irq alloc/free */
- irq_lock_sparse();
+ irq_lock_bitmap();
/* Add the already allocated interrupts */
for_each_irq_desc(irq, desc)
irq_sysfs_add(irq, desc);
- irq_unlock_sparse();
+ irq_unlock_bitmap();
return 0;
}
@@ -607,9 +607,9 @@ static int irq_expand_nr_irqs(unsigned int nr)
void irq_mark_irq(unsigned int irq)
{
- mutex_lock(&sparse_irq_lock);
+ mutex_lock(&bitmap_lock);
bitmap_set(allocated_irqs, irq, 1);
- mutex_unlock(&sparse_irq_lock);
+ mutex_unlock(&bitmap_lock);
}
#ifdef CONFIG_GENERIC_IRQ_LEGACY
@@ -711,14 +711,14 @@ int generic_handle_domain_nmi(struct irq_domain *domain, unsigned int hwirq)
}
#endif
-void irq_lock_sparse(void)
+void irq_lock_bitmap(void)
{
- mutex_lock(&sparse_irq_lock);
+ mutex_lock(&bitmap_lock);
}
-void irq_unlock_sparse(void)
+void irq_unlock_bitmap(void)
{
- mutex_unlock(&sparse_irq_lock);
+ mutex_unlock(&bitmap_lock);
}
/* Dynamic interrupt handling */
@@ -735,12 +735,12 @@ void irq_free_descs(unsigned int from, unsigned int cnt)
if (from >= nr_irqs || (from + cnt) > nr_irqs)
return;
- mutex_lock(&sparse_irq_lock);
+ mutex_lock(&bitmap_lock);
for (i = 0; i < cnt; i++)
free_desc(from + i);
bitmap_clear(allocated_irqs, from, cnt);
- mutex_unlock(&sparse_irq_lock);
+ mutex_unlock(&bitmap_lock);
}
EXPORT_SYMBOL_GPL(irq_free_descs);
@@ -779,7 +779,7 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
from = arch_dynirq_lower_bound(from);
}
- mutex_lock(&sparse_irq_lock);
+ mutex_lock(&bitmap_lock);
start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
from, cnt, 0);
@@ -794,7 +794,7 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
}
ret = alloc_descs(start, cnt, node, affinity, owner);
unlock:
- mutex_unlock(&sparse_irq_lock);
+ mutex_unlock(&bitmap_lock);
return ret;
}
EXPORT_SYMBOL_GPL(__irq_alloc_descs);
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 4b67a4c7de3c..9b81a738d84f 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -133,7 +133,7 @@ void suspend_device_irqs(void)
struct irq_desc *desc;
int irq;
- irq_lock_sparse();
+ irq_lock_bitmap();
for_each_irq_desc(irq, desc) {
unsigned long flags;
bool sync;
@@ -147,7 +147,7 @@ void suspend_device_irqs(void)
if (sync)
synchronize_irq(irq);
}
- irq_unlock_sparse();
+ irq_unlock_bitmap();
}
EXPORT_SYMBOL_GPL(suspend_device_irqs);
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread