* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling [not found] <tip-a5ef2e70405c8a9ee380b5ff33a008c75454791f@git.kernel.org> @ 2010-10-12 21:20 ` Jacob Pan 2010-10-12 21:28 ` Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Jacob Pan @ 2010-10-12 21:20 UTC (permalink / raw) To: linux-kernel, mingo, hpa, tglx, jacob.jun.pan, mingo; +Cc: linux-tip-commits >x86: Sanitize apb timer interrupt handling > >Disable the interrupt in CPU_DEAD where it belongs. My main concern is the performance cost. The power management code for Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) but much more frequently. The system low power states are call S0 idle state (s0ix). Leaving the irq enabled at the chip and desc level between S0ix states might give some performance benefit. That was my original thought. Will it cause problems? Thanks, Jacob ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling 2010-10-12 21:20 ` [tip:irq/core] x86: Sanitize apb timer interrupt handling Jacob Pan @ 2010-10-12 21:28 ` Thomas Gleixner 2010-10-12 22:12 ` Jacob Pan 0 siblings, 1 reply; 6+ messages in thread From: Thomas Gleixner @ 2010-10-12 21:28 UTC (permalink / raw) To: Jacob Pan; +Cc: linux-kernel, mingo, hpa, mingo, linux-tip-commits On Tue, 12 Oct 2010, Jacob Pan wrote: > > >x86: Sanitize apb timer interrupt handling > > > >Disable the interrupt in CPU_DEAD where it belongs. > My main concern is the performance cost. The power management code for > Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) > but much more frequently. The system low power states are call S0 idle > state (s0ix). > > Leaving the irq enabled at the chip and desc level between S0ix states > might give some performance benefit. That was my original thought. > Will it cause problems? Errm. I merily moved it to the place where it should be. You do the disable/enable dance already today. Thanks, tglx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling 2010-10-12 21:28 ` Thomas Gleixner @ 2010-10-12 22:12 ` Jacob Pan 2010-10-12 22:26 ` Thomas Gleixner 0 siblings, 1 reply; 6+ messages in thread From: Jacob Pan @ 2010-10-12 22:12 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linux-kernel, mingo, hpa, mingo, linux-tip-commits Thomas Gleixner Tue, 12 Oct 2010 23:28:19 +0200 (CEST) >On Tue, 12 Oct 2010, Jacob Pan wrote: > >> >> >x86: Sanitize apb timer interrupt handling >> > >> >Disable the interrupt in CPU_DEAD where it belongs. >> My main concern is the performance cost. The power management code for >> Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) >> but much more frequently. The system low power states are call S0 idle >> state (s0ix). >> >> Leaving the irq enabled at the chip and desc level between S0ix states >> might give some performance benefit. That was my original thought. >> Will it cause problems? > >Errm. I merily moved it to the place where it should be. You do the >disable/enable dance already today. > I think I only do disable/enable at the timer HW level today during cpu hp notification, not calling disable_irq(). Am i missing something? Thanks, Jacob ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling 2010-10-12 22:12 ` Jacob Pan @ 2010-10-12 22:26 ` Thomas Gleixner 2010-10-12 22:44 ` Jacob Pan 2010-10-13 18:10 ` Jacob Pan 0 siblings, 2 replies; 6+ messages in thread From: Thomas Gleixner @ 2010-10-12 22:26 UTC (permalink / raw) To: Jacob Pan; +Cc: linux-kernel, mingo, hpa, mingo, linux-tip-commits On Tue, 12 Oct 2010, Jacob Pan wrote: > Thomas Gleixner Tue, 12 Oct 2010 23:28:19 +0200 (CEST) > >On Tue, 12 Oct 2010, Jacob Pan wrote: > > > >> > >> >x86: Sanitize apb timer interrupt handling > >> > > >> >Disable the interrupt in CPU_DEAD where it belongs. > >> My main concern is the performance cost. The power management code for > >> Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) > >> but much more frequently. The system low power states are call S0 idle > >> state (s0ix). > >> > >> Leaving the irq enabled at the chip and desc level between S0ix states > >> might give some performance benefit. That was my original thought. > >> Will it cause problems? > > > >Errm. I merily moved it to the place where it should be. You do the > >disable/enable dance already today. > > > I think I only do disable/enable at the timer HW level today during cpu hp > notification, not calling disable_irq(). Am i missing something? apbt_setup_irq() { ... ---> disable_irq(adev->irq); desc->status |= IRQ_MOVE_PCNTXT; irq_set_affinity(adev->irq, cpumask_of(adev->cpu)); /* APB timer irqs are set up as mp_irqs, timer is edge triggerred */ set_irq_chip_and_handler_name(adev->irq, chip, handle_edge_irq, "edge"); ---> enable_irq(adev->irq); if (system_state == SYSTEM_BOOTING) ... } So that's a disable/enable pair on every cpu hotplug, right ? Now I moved the disable to CPU_DEAD where it belongs and the enable stayed at the same place. Thanks, tglx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling 2010-10-12 22:26 ` Thomas Gleixner @ 2010-10-12 22:44 ` Jacob Pan 2010-10-13 18:10 ` Jacob Pan 1 sibling, 0 replies; 6+ messages in thread From: Jacob Pan @ 2010-10-12 22:44 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linux-kernel, mingo, hpa, mingo, linux-tip-commits Thomas Gleixner Wed, 13 Oct 2010 00:26:08 +0200 (CEST) >On Tue, 12 Oct 2010, Jacob Pan wrote: > >> Thomas Gleixner Tue, 12 Oct 2010 23:28:19 +0200 (CEST) >> >On Tue, 12 Oct 2010, Jacob Pan wrote: >> > >> >> >> >> >x86: Sanitize apb timer interrupt handling >> >> > >> >> >Disable the interrupt in CPU_DEAD where it belongs. >> >> My main concern is the performance cost. The power management code for >> >> Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) >> >> but much more frequently. The system low power states are call S0 idle >> >> state (s0ix). >> >> >> >> Leaving the irq enabled at the chip and desc level between S0ix states >> >> might give some performance benefit. That was my original thought. >> >> Will it cause problems? >> > >> >Errm. I merily moved it to the place where it should be. You do the >> >disable/enable dance already today. >> > >> I think I only do disable/enable at the timer HW level today during cpu hp >> notification, not calling disable_irq(). Am i missing something? > >apbt_setup_irq() >{ > ... > >---> disable_irq(adev->irq); > desc->status |= IRQ_MOVE_PCNTXT; > irq_set_affinity(adev->irq, cpumask_of(adev->cpu)); > /* APB timer irqs are set up as mp_irqs, timer is edge triggerred */ > set_irq_chip_and_handler_name(adev->irq, chip, handle_edge_irq, "edge"); >---> enable_irq(adev->irq); > if (system_state == SYSTEM_BOOTING) > ... >} > >So that's a disable/enable pair on every cpu hotplug, right ? > >Now I moved the disable to CPU_DEAD where it belongs and the enable >stayed at the same place. apbt_setup_irq() does not get called in cpu hotplug code, but CPU_DEAD notification will happen every time disable_nonboot_cpus() are called. Since Moorestown OSPM code uses disable_nonboot_cpus often, that is why i am concerned about the overhead of disable_irq(). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:irq/core] x86: Sanitize apb timer interrupt handling 2010-10-12 22:26 ` Thomas Gleixner 2010-10-12 22:44 ` Jacob Pan @ 2010-10-13 18:10 ` Jacob Pan 1 sibling, 0 replies; 6+ messages in thread From: Jacob Pan @ 2010-10-13 18:10 UTC (permalink / raw) To: Thomas Gleixner; +Cc: linux-kernel, mingo, hpa, mingo, linux-tip-commits Hi tglx, You are right, i am already doing the enable/disable dance. Thanks for pointing it out, patiently. Jacob Thomas Gleixner Wed, 13 Oct 2010 00:26:08 +0200 (CEST) >On Tue, 12 Oct 2010, Jacob Pan wrote: > >> Thomas Gleixner Tue, 12 Oct 2010 23:28:19 +0200 (CEST) >> >On Tue, 12 Oct 2010, Jacob Pan wrote: >> > >> >> >> >> >x86: Sanitize apb timer interrupt handling >> >> > >> >> >Disable the interrupt in CPU_DEAD where it belongs. >> >> My main concern is the performance cost. The power management code for >> >> Moorestown system make use of the cpu hotplug code (disable_nonboot_cpus) >> >> but much more frequently. The system low power states are call S0 idle >> >> state (s0ix). >> >> >> >> Leaving the irq enabled at the chip and desc level between S0ix states >> >> might give some performance benefit. That was my original thought. >> >> Will it cause problems? >> > >> >Errm. I merily moved it to the place where it should be. You do the >> >disable/enable dance already today. >> > >> I think I only do disable/enable at the timer HW level today during cpu hp >> notification, not calling disable_irq(). Am i missing something? > >apbt_setup_irq() >{ > ... > >---> disable_irq(adev->irq); > desc->status |= IRQ_MOVE_PCNTXT; > irq_set_affinity(adev->irq, cpumask_of(adev->cpu)); > /* APB timer irqs are set up as mp_irqs, timer is edge triggerred */ > set_irq_chip_and_handler_name(adev->irq, chip, handle_edge_irq, "edge"); >---> enable_irq(adev->irq); > if (system_state == SYSTEM_BOOTING) > ... >} > >So that's a disable/enable pair on every cpu hotplug, right ? > >Now I moved the disable to CPU_DEAD where it belongs and the enable >stayed at the same place. > >Thanks, > > tglx yo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-13 18:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <tip-a5ef2e70405c8a9ee380b5ff33a008c75454791f@git.kernel.org>
2010-10-12 21:20 ` [tip:irq/core] x86: Sanitize apb timer interrupt handling Jacob Pan
2010-10-12 21:28 ` Thomas Gleixner
2010-10-12 22:12 ` Jacob Pan
2010-10-12 22:26 ` Thomas Gleixner
2010-10-12 22:44 ` Jacob Pan
2010-10-13 18:10 ` Jacob Pan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox