Hi,
In testing we were noticing that we were getting some intermittent
crashes in profile_tick() when dyntick was enabled.
The crashes were because the frame pointer per_cpu____irq_regs value was
0. That code does a user_mode(get_irq_regs()). Currently regs is set
only upon real hardware entry on an irq.
The crash path shows resend_irqs() could be called with in a context
where set_irq_regs() was not executed. In one specific case this was
from
softirq->tasklet_action(resend_tasklet)->resend_irqs->handle_level_irq->
handle_IRQ_event->...->profile_tick.
It seems anyone calling kernel/irq/manage.c:enable_irq() at the wrong
time can trigger this crash.
Creating a fake stack and doing a set_irq_regs() fixes the crash. Would
it be useful to set a pointer to the entry context on all state changes?
For ease I just hacked a default fake stack into the init process after
fork time so there is never a 0 but that doesn't seem so nice.
Regards,
Richard W.
On Wed, 2007-05-16 at 18:20 -0500, Woodruff, Richard wrote:
> The crashes were because the frame pointer per_cpu____irq_regs value was
> 0. That code does a user_mode(get_irq_regs()). Currently regs is set
> only upon real hardware entry on an irq.
>
> The crash path shows resend_irqs() could be called with in a context
> where set_irq_regs() was not executed. In one specific case this was
> from
> softirq->tasklet_action(resend_tasklet)->resend_irqs->handle_level_irq->
> handle_IRQ_event->...->profile_tick.
>
> It seems anyone calling kernel/irq/manage.c:enable_irq() at the wrong
> time can trigger this crash.
which code is disabling / enabling the timer interrupt ?
tglx
> On Wed, 2007-05-16 at 18:20 -0500, Woodruff, Richard wrote:
> > The crashes were because the frame pointer per_cpu____irq_regs value
was
> > 0. That code does a user_mode(get_irq_regs()). Currently regs is
set
> > only upon real hardware entry on an irq.
> >
> > The crash path shows resend_irqs() could be called with in a context
> > where set_irq_regs() was not executed. In one specific case this
was
> > from
> >
softirq->tasklet_action(resend_tasklet)->resend_irqs->handle_level_irq->
> > handle_IRQ_event->...->profile_tick.
> >
> > It seems anyone calling kernel/irq/manage.c:enable_irq() at the
wrong
> > time can trigger this crash.
>
> which code is disabling / enabling the timer interrupt ?
- No one in this case is calling enable_irq(#timer). The failure is
triggered from a non-tick-related enable_irq(#x). The function
handle_IRQ_event() always calls handle_dynamic_tick(). Thus every real
interrupt or fake interrupt though resend_irq will touch the timer code
paths.
To better describe:
-0- Users space does an ioctl to driver
-1- This driver calls enable_irq(#x)
-2- This triggers a check_irq_resend()
-3- This causes a tasklet schedule of the resend_tasklet for #x
-4- This driver later does a spin_unlock_bh
-5- This triggers a check for softirq/tasklets
-6- The resend_tasklet is run and calls desc->handle_irq
-7- This calls handle_level_irq
-8- This calls handle_IRQ_event
-9- This first calls handle_dynamic_tick
-A- This will call though the ticker code to tick update
-B- Finally you die in profile_tick.
-C- Boom in dereference of 0 from user_mode(regs)
As there was no real interrupt the frame marker for irq_regs was not set
and the system dies. Entry was via trap from the ioctl, not irq do_irq.
A dummy non-zero frame allows it to work but doesn't give true
profiling. The resend path seems generally unsafe today. Why not set
it on traps?
Regards,
Richard W.
On Thu, 2007-05-17 at 15:14 -0500, Woodruff, Richard wrote:
> > which code is disabling / enabling the timer interrupt ?
>
> - No one in this case is calling enable_irq(#timer). The failure is
> triggered from a non-tick-related enable_irq(#x). The function
> handle_IRQ_event() always calls handle_dynamic_tick(). Thus every real
> interrupt or fake interrupt though resend_irq will touch the timer code
> paths.
>
> To better describe:
> -0- Users space does an ioctl to driver
> -1- This driver calls enable_irq(#x)
> -2- This triggers a check_irq_resend()
> -3- This causes a tasklet schedule of the resend_tasklet for #x
> -4- This driver later does a spin_unlock_bh
> -5- This triggers a check for softirq/tasklets
> -6- The resend_tasklet is run and calls desc->handle_irq
> -7- This calls handle_level_irq
> -8- This calls handle_IRQ_event
> -9- This first calls handle_dynamic_tick
This is the original ARM dyntick stuff, right ?
The dyntick support on your architecture is broken. Why does it fiddle
with the timer, when the system is not idle ?
This stuff should go away ASAP. A lot of ARMs are already converted to
clock events are using the generic NOHZ implementation, which does not
have those problems at all.
tglx
> This is the original ARM dyntick stuff, right ?
Yes this is a version is not using clocksource.
> The dyntick support on your architecture is broken. Why does it fiddle
> with the timer, when the system is not idle ?
I can't yet run the test sequence on the latest kernel so I'll have to
wait to experiment. A brief look at the new code seems to show a
similar path but I need to actually run though it to understand better.
On the irq_resend() path handle_dynamic_tick() is still called as
before. The difference now is the handler looks like it will jump into
some generic event code in tick-common.c. This still calls
ticker_periodic() which calls profile_tick() in and out of one-shot
mode.
It might be the path still exists or I've just not spent enough time
understanding the real flow.
Thanks for the insight and the nice code.
> This stuff should go away ASAP. A lot of ARMs are already converted to
> clock events are using the generic NOHZ implementation, which does not
> have those problems at all.
Ok. Thanks. I look forward to using that when I can.
Regards,
Richard W.
On Thu, 2007-05-17 at 17:24 -0500, Woodruff, Richard wrote:
> > This is the original ARM dyntick stuff, right ?
>
> Yes this is a version is not using clocksource.
>
> > The dyntick support on your architecture is broken. Why does it fiddle
> > with the timer, when the system is not idle ?
>
> I can't yet run the test sequence on the latest kernel so I'll have to
> wait to experiment. A brief look at the new code seems to show a
> similar path but I need to actually run though it to understand better.
>
>
> On the irq_resend() path handle_dynamic_tick() is still called as
> before.
No. NOHZ makes handle_dynamic_tick() a NOP. handle_dynamic_tick()
depends on CONFIG_NO_IDLE_HZ, which is not used when NOHZ is active.
The problem could only arise, when something would disable/enable the
timer interrupt.
tglx