linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: u.kleine-koenig@pengutronix.de (Uwe Kleine-König)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESENT PATCH] Don't disable irqs in set_next_event and set_mode callbacks
Date: Fri, 27 Nov 2009 11:44:55 +0100	[thread overview]
Message-ID: <20091127104454.GA26570@pengutronix.de> (raw)
In-Reply-To: <20091126113158.GC2393@n2100.arm.linux.org.uk>

Hello,

On Thu, Nov 26, 2009 at 11:31:58AM +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 26, 2009 at 10:50:02AM +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 26, 2009 at 11:26:04AM +0100, Uwe Kleine-K?nig wrote:
> > > These functions are called with irqs already off.
> > > 
> > > AT91RM2000 had a WARN_ON_ONCE if irqs were enabled since Nov 2008 with
> > > noone reporting having hit it.
> > 
> > Can we please start to create some documentation for this, even if it
> > just starts off as "these callbacks are always called with irqs
> > disabled" or some such thing.
> > 
> > I find the generic time stuff extremely difficult to work with, and I
> > suspect I'm not the only one.  This is probably why people like to be
> > sure by having their own IRQ disabling.
> 
> To prove the IRQ-ness of the set_next_event callback, I've traced through
> all the time code and come up with all these possible call paths:
> 
> [...]
> 
> All leaves end in one of four cases:
> 1. a call via dev->event_handler
> 2. a function which uses spin_lock_irqsave before calling the child
> 3. a function which uses local_irq_disable before calling the child
> 4. a call which is #if 0'd out
> 
> So, we can be certain that in cases 2, 3, 4, set_next_event will be
> called with IRQs disabled.  That leaves case 1, which is called from
> the implementations interrupt handling function, or:
> 
> tick_do_broadcast
> +-tick_do_periodic_broadcast
> | `-tick_handle_periodic_broadcast
> |   `- dev->event_handler
> `-tick_handle_oneshot_broadcast
>    `- dev->event_handler
I currently fail to trace where the irqs are disabled, but I have an
at91rm2000 machine and the warning doesn't trigger.
Where are irqs reenabled after exception entry?  Is it before or after
the handler is called?
 
For that machine event_handler is hrtimer_interrupt.  That has an
annotation that it's always called with irqs disabled.

> which basically leaves us with the implementations interrupt handling
> function.  If that always calls the event handler with IRQs disabled,
> then set_next_event will also be called with IRQs disabled.
Thomas, do you care to shed light on this?

If you don't care I suggest to add the same check as for at91rm2000 for
the other platforms and see what happens.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-K?nig            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |

  reply	other threads:[~2009-11-27 10:44 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-21  7:39 [PATCH] Don't disable irqs in set_next_event and set_mode callbacks Uwe Kleine-König
2009-09-21  9:04 ` Russell King - ARM Linux
2009-09-21  9:16   ` Uwe Kleine-König
2009-09-21  9:30     ` Russell King - ARM Linux
2009-09-21 10:48     ` Alessandro Rubini
2009-09-21 12:32 ` Kristoffer Ericson
2009-09-23 19:01   ` Eric Miao
2009-09-23 21:04 ` Remy Bohmer
2009-11-26 10:26 ` [RESENT PATCH] " Uwe Kleine-König
2009-11-26 10:50   ` Russell King - ARM Linux
2009-11-26 11:31     ` Russell King - ARM Linux
2009-11-27 10:44       ` Uwe Kleine-König [this message]
2009-11-27 19:08         ` Thomas Gleixner
2009-11-27 19:58         ` Russell King - ARM Linux
2009-11-27 20:38           ` Uwe Kleine-König
2009-11-27 20:44             ` Russell King - ARM Linux
2009-11-27 21:31               ` Thomas Gleixner
2009-11-27 21:59                 ` Russell King - ARM Linux
2009-11-27 22:20                   ` Thomas Gleixner
2009-11-27 21:10           ` [PATCH] warn about shared irqs requesting IRQF_DISABLED registered with setup_irq Uwe Kleine-König
2009-11-27 22:18             ` Thomas Gleixner
2009-11-28 20:03               ` Uwe Kleine-König
2009-11-28 21:50                 ` Thomas Gleixner
2009-11-28 22:13                   ` David Brownell
2009-11-29  2:31                   ` Jamie Lokier
2009-11-29 10:26                     ` Uwe Kleine-König
2009-11-29 15:18                       ` Jamie Lokier
2009-11-29 15:27                         ` Russell King - ARM Linux
2009-11-30 20:39                           ` Jamie Lokier
2009-11-30  9:28                         ` Uwe Kleine-König
2009-11-30  9:54                         ` Thomas Gleixner
2009-11-28 22:09                 ` David Brownell
2009-11-30 10:47             ` [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED at the right place Uwe Kleine-König
2009-11-30 13:54               ` Get rid of IRQF_DISABLED - (was [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED) Thomas Gleixner
2009-11-30 14:03                 ` Peter Zijlstra
2009-11-30 14:24                   ` Thomas Gleixner
2009-11-30 14:47                     ` Alan Cox
2009-11-30 15:01                       ` Russell King - ARM Linux
2009-11-30 15:32                         ` Alan Cox
2009-11-30 15:43                           ` Russell King - ARM Linux
2009-11-30 20:15                         ` Andrew Victor
2009-11-30 20:53                         ` David Brownell
2009-11-30 20:38                       ` David Brownell
2009-12-01  1:42                         ` Andy Walls
2009-11-30 19:59                     ` Benjamin Herrenschmidt
2009-11-30 21:31                       ` Thomas Gleixner
2009-11-30 21:42                         ` Benjamin Herrenschmidt
2009-11-30 21:54                           ` Thomas Gleixner
2009-11-30 14:37                 ` Russell King - ARM Linux
2009-11-30 14:39                   ` Russell King - ARM Linux
2009-11-30 17:48                     ` Thomas Gleixner
2009-11-30 14:51                   ` Alan Cox
2009-11-30 21:59                     ` Thomas Gleixner
2009-11-30 23:30                       ` Alan Cox
2009-11-30 15:38                   ` Nicolas Pitre
2009-11-30 17:46                   ` Thomas Gleixner
2009-11-30 19:51                 ` Uwe Kleine-König
2009-11-30 21:23                   ` Thomas Gleixner
2009-11-30 20:21               ` [PATCH] genirq: warn about IRQF_SHARED|IRQF_DISABLED at the right place David Brownell
2009-11-30 20:27                 ` Uwe Kleine-König
2010-01-12 15:42               ` [RESEND PATCH] " Uwe Kleine-König
2009-11-26 10:51   ` [RESENT PATCH] Don't disable irqs in set_next_event and set_mode callbacks Eric Miao
2009-12-17 13:33   ` [PATCH 1/2] arm/at91: " Uwe Kleine-König
2009-12-17 13:33     ` [PATCH 2/2] arm/{pxa, sa1100, nomadik}: Don't disable irqs in set_next_event and set_mode Uwe Kleine-König
2010-01-22 16:08     ` [PATCH 1/2] arm/at91: Don't disable irqs in set_next_event and set_mode callbacks Uwe Kleine-König
2010-01-22 16:36       ` Russell King - ARM Linux
2010-01-22 16:52         ` Uwe Kleine-König
2010-02-12 10:35           ` Uwe Kleine-König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091127104454.GA26570@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).