* [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
@ 2013-09-27 0:18 Scott Wood
2013-09-27 15:52 ` Kumar Gala
0 siblings, 1 reply; 5+ messages in thread
From: Scott Wood @ 2013-09-27 0:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev
Otherwise, we get a debug traceback due to the use of
smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
mpic_host_map() is just looking for a default CPU, so it doesn't matter
if we migrate after getting the CPU ID.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/sysdev/mpic.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 1be54fa..bdcb858 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, unsigned int virq,
* is done here.
*/
if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
+ int cpu;
+
+ preempt_disable();
+ cpu = mpic_processor_id(mpic);
+ preempt_enable();
+
mpic_set_vector(virq, hw);
- mpic_set_destination(virq, mpic_processor_id(mpic));
+ mpic_set_destination(virq, cpu);
mpic_irq_set_priority(virq, 8);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
2013-09-27 0:18 [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id() Scott Wood
@ 2013-09-27 15:52 ` Kumar Gala
2013-09-27 16:15 ` Scott Wood
0 siblings, 1 reply; 5+ messages in thread
From: Kumar Gala @ 2013-09-27 15:52 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
> Otherwise, we get a debug traceback due to the use of
> smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
> mpic_host_map() is just looking for a default CPU, so it doesn't =
matter
> if we migrate after getting the CPU ID.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/sysdev/mpic.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>=20
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 1be54fa..bdcb858 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, =
unsigned int virq,
> * is done here.
> */
> if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
> + int cpu;
> +
> + preempt_disable();
> + cpu =3D mpic_processor_id(mpic);
> + preempt_enable();
> +
Any reason you didn't stick this inside of mpic_processor_id() ?
> mpic_set_vector(virq, hw);
> - mpic_set_destination(virq, mpic_processor_id(mpic));
> + mpic_set_destination(virq, cpu);
> mpic_irq_set_priority(virq, 8);
> }
>=20
> --=20
> 1.8.1.2
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
2013-09-27 15:52 ` Kumar Gala
@ 2013-09-27 16:15 ` Scott Wood
2013-09-27 17:09 ` Kumar Gala
0 siblings, 1 reply; 5+ messages in thread
From: Scott Wood @ 2013-09-27 16:15 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
On Fri, 2013-09-27 at 10:52 -0500, Kumar Gala wrote:
> On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
>
> > Otherwise, we get a debug traceback due to the use of
> > smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
> > mpic_host_map() is just looking for a default CPU, so it doesn't matter
> > if we migrate after getting the CPU ID.
> >
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> > ---
> > arch/powerpc/sysdev/mpic.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> > index 1be54fa..bdcb858 100644
> > --- a/arch/powerpc/sysdev/mpic.c
> > +++ b/arch/powerpc/sysdev/mpic.c
> > @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, unsigned int virq,
> > * is done here.
> > */
> > if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
> > + int cpu;
> > +
> > + preempt_disable();
> > + cpu = mpic_processor_id(mpic);
> > + preempt_enable();
> > +
>
> Any reason you didn't stick this inside of mpic_processor_id() ?
Because the debug check might be valid for other callers and we don't
want to defeat it. In this caller it's used only as a heuristic and
thus it doesn't matter if we re-enable preemption before using the
result.
-Scott
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
2013-09-27 16:15 ` Scott Wood
@ 2013-09-27 17:09 ` Kumar Gala
2013-09-27 17:26 ` Scott Wood
0 siblings, 1 reply; 5+ messages in thread
From: Kumar Gala @ 2013-09-27 17:09 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
On Sep 27, 2013, at 11:15 AM, Scott Wood wrote:
> On Fri, 2013-09-27 at 10:52 -0500, Kumar Gala wrote:
>> On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
>>=20
>>> Otherwise, we get a debug traceback due to the use of
>>> smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
>>> mpic_host_map() is just looking for a default CPU, so it doesn't =
matter
>>> if we migrate after getting the CPU ID.
>>>=20
>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>>> ---
>>> arch/powerpc/sysdev/mpic.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>=20
>>> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
>>> index 1be54fa..bdcb858 100644
>>> --- a/arch/powerpc/sysdev/mpic.c
>>> +++ b/arch/powerpc/sysdev/mpic.c
>>> @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain =
*h, unsigned int virq,
>>> * is done here.
>>> */
>>> if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
>>> + int cpu;
>>> +
>>> + preempt_disable();
>>> + cpu =3D mpic_processor_id(mpic);
>>> + preempt_enable();
>>> +
>>=20
>> Any reason you didn't stick this inside of mpic_processor_id() ?
>=20
> Because the debug check might be valid for other callers and we don't
> want to defeat it. In this caller it's used only as a heuristic and
> thus it doesn't matter if we re-enable preemption before using the
> result.
Gotcha, I think you should reword the commit message. Reading it makes =
me think that preemption should be disabled for all mpic_processor_id() =
calls.
- k=
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id()
2013-09-27 17:09 ` Kumar Gala
@ 2013-09-27 17:26 ` Scott Wood
0 siblings, 0 replies; 5+ messages in thread
From: Scott Wood @ 2013-09-27 17:26 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
On Fri, 2013-09-27 at 12:09 -0500, Kumar Gala wrote:
> On Sep 27, 2013, at 11:15 AM, Scott Wood wrote:
>
> > On Fri, 2013-09-27 at 10:52 -0500, Kumar Gala wrote:
> >> On Sep 26, 2013, at 7:18 PM, Scott Wood wrote:
> >>
> >>> Otherwise, we get a debug traceback due to the use of
> >>> smp_processor_id() (or get_paca()) inside hard_smp_processor_id().
> >>> mpic_host_map() is just looking for a default CPU, so it doesn't matter
> >>> if we migrate after getting the CPU ID.
> >>>
> >>> Signed-off-by: Scott Wood <scottwood@freescale.com>
> >>> ---
> >>> arch/powerpc/sysdev/mpic.c | 8 +++++++-
> >>> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> >>> index 1be54fa..bdcb858 100644
> >>> --- a/arch/powerpc/sysdev/mpic.c
> >>> +++ b/arch/powerpc/sysdev/mpic.c
> >>> @@ -1088,8 +1088,14 @@ static int mpic_host_map(struct irq_domain *h, unsigned int virq,
> >>> * is done here.
> >>> */
> >>> if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
> >>> + int cpu;
> >>> +
> >>> + preempt_disable();
> >>> + cpu = mpic_processor_id(mpic);
> >>> + preempt_enable();
> >>> +
> >>
> >> Any reason you didn't stick this inside of mpic_processor_id() ?
> >
> > Because the debug check might be valid for other callers and we don't
> > want to defeat it. In this caller it's used only as a heuristic and
> > thus it doesn't matter if we re-enable preemption before using the
> > result.
>
> Gotcha, I think you should reword the commit message. Reading it makes
> me think that preemption should be disabled for all mpic_processor_id()
> calls.
It should be disabled for all mpic_processor_id() calls -- but some
calls will need preemption to be disabled for longer than just the call
to mpic_processor_id().
I did mention in the commit message why mpic_host_map() is unusual.
BTW, I wonder why we don't use mpic_set_affinity (and thus
irq_choose_cpu), both here and in the non-MPIC_NO_RESET case. Though I
usually notice IRQs being round robinned -- I guess userspace is writing
to the affinity in proc? Or is something elsewhere in the kernel
calling set_affinity at some point?
Another thing I just noticed while looking at IRQ_DESTINATION-related
code, in mpic_teardown_this_cpu(), what stops it from removing the only
CPU that an interrupt is targeted at?
-Scott
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-27 17:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 0:18 [PATCH] powerpc/mpic: Disable preemption when calling mpic_processor_id() Scott Wood
2013-09-27 15:52 ` Kumar Gala
2013-09-27 16:15 ` Scott Wood
2013-09-27 17:09 ` Kumar Gala
2013-09-27 17:26 ` Scott Wood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox