| Message ID | 20240130082722.2912576-4-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | irqchip/loongson-eiointc: Refine irq affinity setting during resume | expand |
On Tue, Jan 30 2024 at 16:27, Bibo Mao wrote: > During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs > will be migrated to CPU0. So it is not necessary to restore irq affinity > for eiointc irq controller when system resumes. That's not the reason. The point is that eiointc_router_init() which is invoked in the resume path affines all interrupts to CPU0, so the restore operation is redundant, no? > This patch removes this piece of code about irq affinity restoring in > function eiointc_resume(). Again. 'This patch' is pointless because we already know that this is a patch, no?
Hi Thomas, On 2024/2/13 下午5:49, Thomas Gleixner wrote: > On Tue, Jan 30 2024 at 16:27, Bibo Mao wrote: >> During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs >> will be migrated to CPU0. So it is not necessary to restore irq affinity >> for eiointc irq controller when system resumes. > > That's not the reason. The point is that eiointc_router_init() which is > invoked in the resume path affines all interrupts to CPU0, so the > restore operation is redundant, no? yes, it is. eiointc_router_init() will enable the irqs and affine all interrupts to CPU0. And it is redundant, the aim of deleted code wants to resume older interrupt affinity when executing "echo mem > /sys/power/state". > >> This patch removes this piece of code about irq affinity restoring in >> function eiointc_resume(). > > Again. 'This patch' is pointless because we already know that this is a > patch, no? Thanks for your kindly help, English is somewhat difficult for me :) Regards Bibo Mao
Hi, Thomas, On Tue, Feb 13, 2024 at 5:49 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Tue, Jan 30 2024 at 16:27, Bibo Mao wrote: > > During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs > > will be migrated to CPU0. So it is not necessary to restore irq affinity > > for eiointc irq controller when system resumes. > > That's not the reason. The point is that eiointc_router_init() which is > invoked in the resume path affines all interrupts to CPU0, so the > restore operation is redundant, no? I'm sorry for the late response but I think this is a little wrong. When irq_migrate_all_off_this_cpu() is called at hot-unplug, if an irqdesc is irqd_affinity_is_managed() then its affinity is untouched (doesn't change to CPU0). Then after resume we should not keep its affinity on CPU0 set by eiointc_router_init() , but need to restore its old affinity. Huacai > > > This patch removes this piece of code about irq affinity restoring in > > function eiointc_resume(). > > Again. 'This patch' is pointless because we already know that this is a > patch, no? >
On Wed, Mar 13 2024 at 14:20, Huacai Chen wrote: > On Tue, Feb 13, 2024 at 5:49 PM Thomas Gleixner <tglx@linutronix.de> wrote: >> >> On Tue, Jan 30 2024 at 16:27, Bibo Mao wrote: >> > During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs >> > will be migrated to CPU0. So it is not necessary to restore irq affinity >> > for eiointc irq controller when system resumes. >> >> That's not the reason. The point is that eiointc_router_init() which is >> invoked in the resume path affines all interrupts to CPU0, so the >> restore operation is redundant, no? > I'm sorry for the late response but I think this is a little wrong. > When irq_migrate_all_off_this_cpu() is called at hot-unplug, if an > irqdesc is irqd_affinity_is_managed() then its affinity is untouched > (doesn't change to CPU0). Then after resume we should not keep its > affinity on CPU0 set by eiointc_router_init() , but need to restore > its old affinity. Affinity is restored when the interrupt is started up again, so yes the affinity setting should not be changed.
On Wed, Mar 13, 2024 at 8:39 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Wed, Mar 13 2024 at 14:20, Huacai Chen wrote: > > On Tue, Feb 13, 2024 at 5:49 PM Thomas Gleixner <tglx@linutronix.de> wrote: > >> > >> On Tue, Jan 30 2024 at 16:27, Bibo Mao wrote: > >> > During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs > >> > will be migrated to CPU0. So it is not necessary to restore irq affinity > >> > for eiointc irq controller when system resumes. > >> > >> That's not the reason. The point is that eiointc_router_init() which is > >> invoked in the resume path affines all interrupts to CPU0, so the > >> restore operation is redundant, no? > > I'm sorry for the late response but I think this is a little wrong. > > When irq_migrate_all_off_this_cpu() is called at hot-unplug, if an > > irqdesc is irqd_affinity_is_managed() then its affinity is untouched > > (doesn't change to CPU0). Then after resume we should not keep its > > affinity on CPU0 set by eiointc_router_init() , but need to restore > > its old affinity. > > Affinity is restored when the interrupt is started up again, so yes the > affinity setting should not be changed. OK, thanks. Huacai
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c index 6a71a8c29ac7..b64cbe3052e8 100644 --- a/drivers/irqchip/irq-loongson-eiointc.c +++ b/drivers/irqchip/irq-loongson-eiointc.c @@ -310,23 +310,7 @@ static int eiointc_suspend(void) static void eiointc_resume(void) { - int i, j; - struct irq_desc *desc; - struct irq_data *irq_data; - eiointc_router_init(0); - - for (i = 0; i < nr_pics; i++) { - for (j = 0; j < eiointc_priv[0]->vec_count; j++) { - desc = irq_resolve_mapping(eiointc_priv[i]->eiointc_domain, j); - if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) { - raw_spin_lock(&desc->lock); - irq_data = irq_domain_get_irq_data(eiointc_priv[i]->eiointc_domain, irq_desc_get_irq(desc)); - eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0); - raw_spin_unlock(&desc->lock); - } - } - } } static struct syscore_ops eiointc_syscore_ops = {
During suspend and resume, CPUs except CPU0 can be hot-unpluged and IRQs will be migrated to CPU0. So it is not necessary to restore irq affinity for eiointc irq controller when system resumes. This patch removes this piece of code about irq affinity restoring in function eiointc_resume(). Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- drivers/irqchip/irq-loongson-eiointc.c | 16 ---------------- 1 file changed, 16 deletions(-)