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 <scottw...@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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
