* [PATCH 1/3] HPET: Convert WARN_ON to WARN_ON_ONCE @ 2008-11-02 16:04 Matt Fleming 2008-11-02 16:04 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Matt Fleming 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2008-11-02 16:04 UTC (permalink / raw) To: linux-kernel; +Cc: tglx, mingo, venkatesh.pallipadi, Matt Fleming It is possible to flood the console with call traces if the WARN_ON condition is true because of the frequency with which this function is called. Signed-off-by: Matt Fleming <mjf@gentoo.org> --- arch/x86/kernel/hpet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index 77017e8..f10f946 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -322,7 +322,7 @@ static int hpet_next_event(unsigned long delta, * what we wrote hit the chip before we compare it to the * counter. */ - WARN_ON((u32)hpet_readl(HPET_T0_CMP) != cnt); + WARN_ON_ONCE((u32)hpet_readl(HPET_T0_CMP) != cnt); return (s32)((u32)hpet_readl(HPET_COUNTER) - cnt) >= 0 ? -ETIME : 0; } -- 1.5.6.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled 2008-11-02 16:04 [PATCH 1/3] HPET: Convert WARN_ON to WARN_ON_ONCE Matt Fleming @ 2008-11-02 16:04 ` Matt Fleming 2008-11-02 16:04 ` [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP Matt Fleming 2008-11-02 21:35 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Will Newton 0 siblings, 2 replies; 8+ messages in thread From: Matt Fleming @ 2008-11-02 16:04 UTC (permalink / raw) To: linux-kernel; +Cc: tglx, mingo, venkatesh.pallipadi, Matt Fleming Some functions that may be called from this handler require that interrupts are disabled. Signed-off-by: Matt Fleming <mjf@gentoo.org> --- arch/x86/kernel/hpet.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index f10f946..c28fff2 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -445,7 +445,8 @@ static int hpet_setup_irq(struct hpet_dev *dev) { if (request_irq(dev->irq, hpet_interrupt_handler, - IRQF_SHARED|IRQF_NOBALANCING, dev->name, dev)) + IRQF_DISABLED|IRQF_SHARED|IRQF_NOBALANCING, + dev->name, dev)) return -1; disable_irq(dev->irq); -- 1.5.6.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP 2008-11-02 16:04 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Matt Fleming @ 2008-11-02 16:04 ` Matt Fleming 2008-11-09 10:33 ` Matt Fleming 2008-11-02 21:35 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Will Newton 1 sibling, 1 reply; 8+ messages in thread From: Matt Fleming @ 2008-11-02 16:04 UTC (permalink / raw) To: linux-kernel; +Cc: tglx, mingo, venkatesh.pallipadi, Matt Fleming In hpet_next_event() we check that the value we just wrote to HPET_Tn_CMP(timer) has reached the chip. Currently, we're checking that the value we wrote to HPET_Tn_CMP(timer) is in HPET_T0_CMP, which, if timer is anything other than timer 0, is likely to fail. Signed-off-by: Matt Fleming <mjf@gentoo.org> --- arch/x86/kernel/hpet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index c28fff2..76feb2e 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -322,7 +322,7 @@ static int hpet_next_event(unsigned long delta, * what we wrote hit the chip before we compare it to the * counter. */ - WARN_ON_ONCE((u32)hpet_readl(HPET_T0_CMP) != cnt); + WARN_ON_ONCE((u32)hpet_readl(HPET_Tn_CMP(timer)) != cnt); return (s32)((u32)hpet_readl(HPET_COUNTER) - cnt) >= 0 ? -ETIME : 0; } -- 1.5.6.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP 2008-11-02 16:04 ` [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP Matt Fleming @ 2008-11-09 10:33 ` Matt Fleming 2008-11-10 16:40 ` Thomas Gleixner 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2008-11-09 10:33 UTC (permalink / raw) To: linux-kernel; +Cc: tglx, mingo, venkatesh.pallipadi 2008/11/2 Matt Fleming <mjf@gentoo.org>: > In hpet_next_event() we check that the value we just wrote to > HPET_Tn_CMP(timer) has reached the chip. Currently, we're checking that > the value we wrote to HPET_Tn_CMP(timer) is in HPET_T0_CMP, which, if > timer is anything other than timer 0, is likely to fail. > Any comments on this set of patches? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP 2008-11-09 10:33 ` Matt Fleming @ 2008-11-10 16:40 ` Thomas Gleixner 0 siblings, 0 replies; 8+ messages in thread From: Thomas Gleixner @ 2008-11-10 16:40 UTC (permalink / raw) To: Matt Fleming; +Cc: linux-kernel, mingo, venkatesh.pallipadi Matt, On Sun, 9 Nov 2008, Matt Fleming wrote: > 2008/11/2 Matt Fleming <mjf@gentoo.org>: > > In hpet_next_event() we check that the value we just wrote to > > HPET_Tn_CMP(timer) has reached the chip. Currently, we're checking that > > the value we wrote to HPET_Tn_CMP(timer) is in HPET_T0_CMP, which, if > > timer is anything other than timer 0, is likely to fail. > > > > Any comments on this set of patches? good catch. Applied to tip timers/hpet and scheduled for linus. Thanks, tglx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled 2008-11-02 16:04 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Matt Fleming 2008-11-02 16:04 ` [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP Matt Fleming @ 2008-11-02 21:35 ` Will Newton 2008-11-02 22:23 ` Matt Fleming 1 sibling, 1 reply; 8+ messages in thread From: Will Newton @ 2008-11-02 21:35 UTC (permalink / raw) To: Matt Fleming; +Cc: linux-kernel, tglx, mingo, venkatesh.pallipadi On Sun, Nov 2, 2008 at 4:04 PM, Matt Fleming <mjf@gentoo.org> wrote: > Some functions that may be called from this handler require that > interrupts are disabled. > > Signed-off-by: Matt Fleming <mjf@gentoo.org> > --- > arch/x86/kernel/hpet.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c > index f10f946..c28fff2 100644 > --- a/arch/x86/kernel/hpet.c > +++ b/arch/x86/kernel/hpet.c > @@ -445,7 +445,8 @@ static int hpet_setup_irq(struct hpet_dev *dev) > { > > if (request_irq(dev->irq, hpet_interrupt_handler, > - IRQF_SHARED|IRQF_NOBALANCING, dev->name, dev)) > + IRQF_DISABLED|IRQF_SHARED|IRQF_NOBALANCING, > + dev->name, dev)) Combining IRQF_DISABLED and IRQF_SHARED does not reliably disable interrupts in the handler. Perhaps IRQF_SHARED should be removed at the same time? > return -1; > > disable_irq(dev->irq); > -- > 1.5.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled 2008-11-02 21:35 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Will Newton @ 2008-11-02 22:23 ` Matt Fleming 2008-11-10 16:53 ` Takashi Iwai 0 siblings, 1 reply; 8+ messages in thread From: Matt Fleming @ 2008-11-02 22:23 UTC (permalink / raw) To: Will Newton; +Cc: linux-kernel, tglx, mingo, venkatesh.pallipadi [-- Attachment #1: Type: text/plain, Size: 1569 bytes --] 2008/11/2 Will Newton <will.newton@gmail.com>: > > Combining IRQF_DISABLED and IRQF_SHARED does not reliably disable > interrupts in the handler. Perhaps IRQF_SHARED should be removed at > the same time? > I didn't know that. Under what conditions is it unreliable? I've attached a second attempt at this patch. It seems that the IRQ is never shared anyway, so I followed your suggestion and removed IRQF_SHARED. My machine seems to be running fine. >From c445728f36de599770088e345fc03ef9fafd8470 Mon Sep 17 00:00:00 2001 From: Matt Fleming <mjf@gentoo.org> Date: Sun, 2 Nov 2008 14:00:23 +0000 Subject: [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Some functions that may be called from this handler require that interrupts are disabled. Also, combining IRQF_DISABLED and IRQF_SHARED does not reliably disable interrupts in a handler, so remove IRQF_SHARED from the irq flags (this irq is not shared anyway). Signed-off-by: Matt Fleming <mjf@gentoo.org> --- arch/x86/kernel/hpet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index f10f946..bfb17f4 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -445,7 +445,7 @@ static int hpet_setup_irq(struct hpet_dev *dev) { if (request_irq(dev->irq, hpet_interrupt_handler, - IRQF_SHARED|IRQF_NOBALANCING, dev->name, dev)) + IRQF_DISABLED|IRQF_NOBALANCING, dev->name, dev)) return -1; disable_irq(dev->irq); -- 1.5.6.4 [-- Attachment #2: 0002-HPET-Enter-hpet_interrupt_handler-with-interrupts-d.patch --] [-- Type: application/octet-stream, Size: 1054 bytes --] From c445728f36de599770088e345fc03ef9fafd8470 Mon Sep 17 00:00:00 2001 From: Matt Fleming <mjf@gentoo.org> Date: Sun, 2 Nov 2008 14:00:23 +0000 Subject: [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Some functions that may be called from this handler require that interrupts are disabled. Also, combining IRQF_DISABLED and IRQF_SHARED does not reliably disable interrupts in a handler, so remove IRQF_SHARED from the irq flags (this irq is not shared anyway). Signed-off-by: Matt Fleming <mjf@gentoo.org> --- arch/x86/kernel/hpet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index f10f946..bfb17f4 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -445,7 +445,7 @@ static int hpet_setup_irq(struct hpet_dev *dev) { if (request_irq(dev->irq, hpet_interrupt_handler, - IRQF_SHARED|IRQF_NOBALANCING, dev->name, dev)) + IRQF_DISABLED|IRQF_NOBALANCING, dev->name, dev)) return -1; disable_irq(dev->irq); -- 1.5.6.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled 2008-11-02 22:23 ` Matt Fleming @ 2008-11-10 16:53 ` Takashi Iwai 0 siblings, 0 replies; 8+ messages in thread From: Takashi Iwai @ 2008-11-10 16:53 UTC (permalink / raw) To: Matt Fleming; +Cc: Will Newton, linux-kernel, tglx, mingo, venkatesh.pallipadi At Sun, 2 Nov 2008 22:23:13 +0000, Matt Fleming wrote: > > 2008/11/2 Will Newton <will.newton@gmail.com>: > > > > Combining IRQF_DISABLED and IRQF_SHARED does not reliably disable > > interrupts in the handler. Perhaps IRQF_SHARED should be removed at > > the same time? > > > > I didn't know that. Under what conditions is it unreliable? The kernel checks IRQF_DISABLED bit of only the first handler even if multiple handlers are assigned to a single IRQ line. Thus, if another handler without IRQF_DISABLED has been already registered before yours, your IRQF_DISABLED is ignored. See kernel/irq/handle.c. Takashi ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-11-10 16:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-11-02 16:04 [PATCH 1/3] HPET: Convert WARN_ON to WARN_ON_ONCE Matt Fleming 2008-11-02 16:04 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Matt Fleming 2008-11-02 16:04 ` [PATCH 3/3] HPET: Read from HPET_Tn_CMP() not HPET_T0_CMP Matt Fleming 2008-11-09 10:33 ` Matt Fleming 2008-11-10 16:40 ` Thomas Gleixner 2008-11-02 21:35 ` [PATCH 2/3] HPET: Enter hpet_interrupt_handler with interrupts disabled Will Newton 2008-11-02 22:23 ` Matt Fleming 2008-11-10 16:53 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®