Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283711 > unrolled thread

Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-12-04 12:10 +0100
    Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-12-05 18:30 +0100
      Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-12-06 10:30 +0100
        Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2015-12-08 10:00 +0100
          Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()  removal Thomas Gleixner <tglx@linutronix.de> - 2015-12-08 12:00 +0100

#1283711 — Re: [PATCH 0/5] Fix regression introduced by set_irq_flags() removal

Thomas,

On Fri, 13 Nov 2015 15:11:16 -0500 (EST), Thomas Gleixner wrote:

> On Wed, 11 Nov 2015, Thomas Petazzoni wrote:
> > Have you had the time to consider the proposed solution? For 4.3 we
> > implemented the quick work-around that consisted in clearing
> > IRQ_NOAUTOEN, but it's probably not a very good long-term solution.
> > 
> > Don't hesitate to let me know if you'd like to see some modifications
> > to the proposed approach, or if you have a totally different approach
> > in mind.
> 
> I'm not sure if we really need all that muck if we can just rely on
> that flag. I don't see the extra value, but you might have something
> in mind which does not jump into my face right now.

Well, the problem is that IRQ_NOAUTOEN is a global flag, which is OK
for global interrupts, but not good for per-CPU interrupts, since you
don't have the information on a per-CPU basis of which interrupt was
enabled before suspend, and therefore should be re-enabled after resume.

Until now, we don't have the problem since the only per-CPU interrupt
we were using was the local timer interrupt, and the local timers on
secondary CPUs are switched off during suspend and re-enabled during
resume. So re-enabling the interrupt on the boot CPU on resume is
sufficient.

However, our network driver recently switched to using per-CPU
interrupts as well, and in this case, it is really important to be able
to re-enable the per-CPU interrupts and the appropriate CPUs at resume
time. Since our HW registers are made so that it is not possible to
read out at suspend time which interrupts are enabled, we have to ask
the Linux kernel at resume time which interrupts should be re-enabled
at the HW level. Which is what my more complicated series was doing.

Do you have other suggestions to allow us to know which per-CPU
interrupts should be re-enabled on the different CPUs at resume time ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1284607

Thomas,

On Fri, 4 Dec 2015, Thomas Petazzoni wrote:
> Well, the problem is that IRQ_NOAUTOEN is a global flag, which is OK
> for global interrupts, but not good for per-CPU interrupts, since you
> don't have the information on a per-CPU basis of which interrupt was
> enabled before suspend, and therefore should be re-enabled after resume.
> 
> Until now, we don't have the problem since the only per-CPU interrupt
> we were using was the local timer interrupt, and the local timers on
> secondary CPUs are switched off during suspend and re-enabled during
> resume. So re-enabling the interrupt on the boot CPU on resume is
> sufficient.
> 
> However, our network driver recently switched to using per-CPU
> interrupts as well, and in this case, it is really important to be able
> to re-enable the per-CPU interrupts and the appropriate CPUs at resume
> time. Since our HW registers are made so that it is not possible to
> read out at suspend time which interrupts are enabled, we have to ask
> the Linux kernel at resume time which interrupts should be re-enabled
> at the HW level. Which is what my more complicated series was doing.
> 
> Do you have other suggestions to allow us to know which per-CPU
> interrupts should be re-enabled on the different CPUs at resume time ?

Ok. That makes sense. So I'm going to pick up the core change.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284819

Thomas,

On Sat, 5 Dec 2015, Thomas Gleixner wrote:
> On Fri, 4 Dec 2015, Thomas Petazzoni wrote:
> > Well, the problem is that IRQ_NOAUTOEN is a global flag, which is OK
> > for global interrupts, but not good for per-CPU interrupts, since you
> > don't have the information on a per-CPU basis of which interrupt was
> > enabled before suspend, and therefore should be re-enabled after resume.
> > 
> > Until now, we don't have the problem since the only per-CPU interrupt
> > we were using was the local timer interrupt, and the local timers on
> > secondary CPUs are switched off during suspend and re-enabled during
> > resume. So re-enabling the interrupt on the boot CPU on resume is
> > sufficient.
> > 
> > However, our network driver recently switched to using per-CPU
> > interrupts as well, and in this case, it is really important to be able
> > to re-enable the per-CPU interrupts and the appropriate CPUs at resume
> > time. Since our HW registers are made so that it is not possible to
> > read out at suspend time which interrupts are enabled, we have to ask
> > the Linux kernel at resume time which interrupts should be re-enabled
> > at the HW level. Which is what my more complicated series was doing.
> > 
> > Do you have other suggestions to allow us to know which per-CPU
> > interrupts should be re-enabled on the different CPUs at resume time ?
> 
> Ok. That makes sense. So I'm going to pick up the core change.

Second thoughts. That network driver example does not make sense.

You have a suspend/resume mechanism and a cpu hotplug machinery in
that driver, right? So that should be responsible for
disabling/enabling the per cpu interrupts. I don't think it's the
proper way to do that in the irq chip driver at some random point
during resume as you'd reenable interrupts on cpus which are not
online yet.

Thanks,

	tglx




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1286333

Hello Thomas,

On Sun, 6 Dec 2015 10:28:15 +0100 (CET), Thomas Gleixner wrote:

> Second thoughts. That network driver example does not make sense.
> 
> You have a suspend/resume mechanism and a cpu hotplug machinery in
> that driver, right? So that should be responsible for
> disabling/enabling the per cpu interrupts. I don't think it's the
> proper way to do that in the irq chip driver at some random point
> during resume as you'd reenable interrupts on cpus which are not
> online yet.

The irqchip driver would re-enable the per-CPU interrupts in a CPU
notifier, so only when the secondary CPUs come online again after
resume.

When a device driver uses a normal (non per-CPU) interrupt, then it
doesn't have to take care of disabling the interrupt on suspend and
re-enabling the interrupt on resume at the interrupt controller level.
This is all transparently handled by the irqchip driver.

Why should the handling of per-CPU interrupts be different and require
explicit handling from each device driver rather than being
transparently handled by the irqchip driver ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1286390

On Tue, 8 Dec 2015, Thomas Petazzoni wrote:
> When a device driver uses a normal (non per-CPU) interrupt, then it
> doesn't have to take care of disabling the interrupt on suspend and
> re-enabling the interrupt on resume at the interrupt controller level.
> This is all transparently handled by the irqchip driver.
> 
> Why should the handling of per-CPU interrupts be different and require
> explicit handling from each device driver rather than being
> transparently handled by the irqchip driver ?

Fair enough. Did not think about the boot cpu part.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web