* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
2013-11-06 11:30 ` Thomas Gleixner
@ 2013-11-07 7:59 ` Michael Schmitz
2013-11-10 8:33 ` Michael Schmitz
2013-11-13 19:42 ` [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts tip-bot for Thomas Gleixner
2 siblings, 0 replies; 8+ messages in thread
From: Michael Schmitz @ 2013-11-07 7:59 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven
Thomas,
>>>> This is a temporary hack to allow timer based polling of the
>>>> Atari ROM port network and USB cards only. Suggestions on how to
>>>> properly handle this in the normal interrupt framework are most
>>>> welcome.
>
> So you're polling devices which have no hardware interrupt from the
> timer interrupt. Of course if there is no interrupt pending on one of
> these devices, this will trigger the spurious detector. By setting the
> POLL_INPROGRESS flag, you're preventing that.
That's the general idea, yes. I am aware I abuse the flag somewhat.
This was
the least invasive fix I came up with (i.e. no risk to cause
regressions elsewhere).
Adding a new IRQ flag would have been my preference, but that seemed a
bit
bold at the time.
> Reading the demultiplex handler it seems you have no way to figure out
> which of the sub interrupts actually triggered the mfptimer_handler,
> right?
Correct.
> I'm not too happy about the POLL flag "abuse". I'd rather have such
Precisely why I asked for this to be reviewed, thanks,
> interrupts explicitely marked as polled by some other interrupt. That
> also excludes such interrupts from the spurious mechanism completely.
>
> Does the following patch solve the problem? You need to call
>
> irq_set_status_flags(irq, IRQ_IS_POLLED);
>
> when setting up the interrupt controller for those polled interrupt
> lines.
This should do the trick, from what I can see. I have tried to test it,
but recent changes
to the m68k vmlinux.lds have thrown a monkey wrench in the works, it
seems (kernels
just won't boot at all). I'll sort that out with Geert and report back.
Much obliged,
Michael
>
> Thanks,
>
> tglx
> -----------------
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 56bb0dc..7dc1003 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -70,6 +70,9 @@ typedef void (*irq_preflow_handler_t)(struct
> irq_data *data);
> * IRQ_MOVE_PCNTXT - Interrupt can be migrated from process context
> * IRQ_NESTED_TRHEAD - Interrupt nests into another thread
> * IRQ_PER_CPU_DEVID - Dev_id is a per-cpu variable
> + * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
> + * it from the spurious interrupt detection
> + * mechanism and from core side polling.
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -94,12 +97,14 @@ enum {
> IRQ_NESTED_THREAD = (1 << 15),
> IRQ_NOTHREAD = (1 << 16),
> IRQ_PER_CPU_DEVID = (1 << 17),
> + IRQ_IS_POLLED = (1 << 18),
> };
>
> #define IRQF_MODIFY_MASK \
> (IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
> IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
> - IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
> + IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID
> | \
> + IRQ_IS_POLLED)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index 1162f10..3320b84 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -14,6 +14,7 @@ enum {
> _IRQ_NO_BALANCING = IRQ_NO_BALANCING,
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> + _IRQ_IS_POLLED = IRQ_IS_POLLED,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -26,6 +27,7 @@ enum {
> #define IRQ_NOAUTOEN GOT_YOU_MORON
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> +#define IRQ_IS_POLLED GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -147,3 +149,8 @@ static inline bool
> irq_settings_is_nested_thread(struct irq_desc *desc)
> {
> return desc->status_use_accessors & _IRQ_NESTED_THREAD;
> }
> +
> +static inline bool irq_settings_is_polled(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors & _IRQ_IS_POLLED;
> +}
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 7b5f012..a1d8cc6 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc
> *desc, bool force)
>
> raw_spin_lock(&desc->lock);
>
> - /* PER_CPU and nested thread interrupts are never polled */
> - if (irq_settings_is_per_cpu(desc) ||
> irq_settings_is_nested_thread(desc))
> + /*
> + * PER_CPU, nested thread interrupts and interrupts explicitely
> + * marked polled are excluded from polling.
> + */
> + if (irq_settings_is_per_cpu(desc) ||
> + irq_settings_is_nested_thread(desc) ||
> + irq_settings_is_polled(desc))
> goto out;
>
> /*
> @@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct
> irq_desc *desc,
> void note_interrupt(unsigned int irq, struct irq_desc *desc,
> irqreturn_t action_ret)
> {
> - if (desc->istate & IRQS_POLL_INPROGRESS)
> + if (desc->istate & IRQS_POLL_INPROGRESS ||
> + irq_settings_is_polled(desc))
> return;
>
> /* we get here again via the threaded handler */
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
2013-11-06 11:30 ` Thomas Gleixner
2013-11-07 7:59 ` Michael Schmitz
@ 2013-11-10 8:33 ` Michael Schmitz
2013-11-12 19:08 ` Thomas Gleixner
2013-11-13 19:42 ` [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts tip-bot for Thomas Gleixner
2 siblings, 1 reply; 8+ messages in thread
From: Michael Schmitz @ 2013-11-10 8:33 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven
Thomas,
> Does the following patch solve the problem? You need to call
>
> irq_set_status_flags(irq, IRQ_IS_POLLED);
>
> when setting up the interrupt controller for those polled interrupt
> lines.
Your patch works just fine, thanks.
I suppose setting the flag can be done in the corresponding irq startup
function, instead of when setting up the irq controller?
Geert - I will send the patch to ataints.c implementing this as soon as
Thomas' fix is merged.
Regards,
Michael
> Thanks,
>
> tglx
> -----------------
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 56bb0dc..7dc1003 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -70,6 +70,9 @@ typedef void (*irq_preflow_handler_t)(struct
> irq_data *data);
> * IRQ_MOVE_PCNTXT - Interrupt can be migrated from process context
> * IRQ_NESTED_TRHEAD - Interrupt nests into another thread
> * IRQ_PER_CPU_DEVID - Dev_id is a per-cpu variable
> + * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
> + * it from the spurious interrupt detection
> + * mechanism and from core side polling.
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -94,12 +97,14 @@ enum {
> IRQ_NESTED_THREAD = (1 << 15),
> IRQ_NOTHREAD = (1 << 16),
> IRQ_PER_CPU_DEVID = (1 << 17),
> + IRQ_IS_POLLED = (1 << 18),
> };
>
> #define IRQF_MODIFY_MASK \
> (IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
> IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
> - IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
> + IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID
> | \
> + IRQ_IS_POLLED)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
> index 1162f10..3320b84 100644
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -14,6 +14,7 @@ enum {
> _IRQ_NO_BALANCING = IRQ_NO_BALANCING,
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> + _IRQ_IS_POLLED = IRQ_IS_POLLED,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -26,6 +27,7 @@ enum {
> #define IRQ_NOAUTOEN GOT_YOU_MORON
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> +#define IRQ_IS_POLLED GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -147,3 +149,8 @@ static inline bool
> irq_settings_is_nested_thread(struct irq_desc *desc)
> {
> return desc->status_use_accessors & _IRQ_NESTED_THREAD;
> }
> +
> +static inline bool irq_settings_is_polled(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors & _IRQ_IS_POLLED;
> +}
> diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
> index 7b5f012..a1d8cc6 100644
> --- a/kernel/irq/spurious.c
> +++ b/kernel/irq/spurious.c
> @@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc
> *desc, bool force)
>
> raw_spin_lock(&desc->lock);
>
> - /* PER_CPU and nested thread interrupts are never polled */
> - if (irq_settings_is_per_cpu(desc) ||
> irq_settings_is_nested_thread(desc))
> + /*
> + * PER_CPU, nested thread interrupts and interrupts explicitely
> + * marked polled are excluded from polling.
> + */
> + if (irq_settings_is_per_cpu(desc) ||
> + irq_settings_is_nested_thread(desc) ||
> + irq_settings_is_polled(desc))
> goto out;
>
> /*
> @@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct
> irq_desc *desc,
> void note_interrupt(unsigned int irq, struct irq_desc *desc,
> irqreturn_t action_ret)
> {
> - if (desc->istate & IRQS_POLL_INPROGRESS)
> + if (desc->istate & IRQS_POLL_INPROGRESS ||
> + irq_settings_is_polled(desc))
> return;
>
> /* we get here again via the threaded handler */
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
2013-11-10 8:33 ` Michael Schmitz
@ 2013-11-12 19:08 ` Thomas Gleixner
2013-11-13 7:57 ` Michael Schmitz
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2013-11-12 19:08 UTC (permalink / raw)
To: Michael Schmitz; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven
On Sun, 10 Nov 2013, Michael Schmitz wrote:
> Your patch works just fine, thanks.
>
> I suppose setting the flag can be done in the corresponding irq startup
> function, instead of when setting up the irq controller?
irq_startup() is called with irq_desc->lock held and
irq_set_status_flags() wants desc->lock as well. Deadlock
And no, you don't want to fiddle manually in the irq descriptor data
fields. See commit a6967caf00eb :)
> Geert - I will send the patch to ataints.c implementing this as soon as
> Thomas' fix is merged.
I'll expedite it for 3.13.
Thanks,
tglx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 09/17] [m68k] IRQ: add handle_polled_irq() for timer based soft interrupts
2013-11-12 19:08 ` Thomas Gleixner
@ 2013-11-13 7:57 ` Michael Schmitz
0 siblings, 0 replies; 8+ messages in thread
From: Michael Schmitz @ 2013-11-13 7:57 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-m68k, linux-kernel, Geert Uytterhoeven
Thomas,
>> I suppose setting the flag can be done in the corresponding irq
>> startup
>> function, instead of when setting up the irq controller?
>
> irq_startup() is called with irq_desc->lock held and
> irq_set_status_flags() wants desc->lock as well. Deadlock
Thanks, point taken.
> And no, you don't want to fiddle manually in the irq descriptor data
> fields. See commit a6967caf00eb :)
I'm not that crazy :-)
>
>> Geert - I will send the patch to ataints.c implementing this as soon
>> as
>> Thomas' fix is merged.
>
> I'll expedite it for 3.13.
Thanks, that'll help.
Cheers,
Michael
>
> Thanks,
>
> tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:irq/urgent] genirq: Prevent spurious detection for unconditionally polled interrupts
2013-11-06 11:30 ` Thomas Gleixner
2013-11-07 7:59 ` Michael Schmitz
2013-11-10 8:33 ` Michael Schmitz
@ 2013-11-13 19:42 ` tip-bot for Thomas Gleixner
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Thomas Gleixner @ 2013-11-13 19:42 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, geert, tglx, schmitzmic
Commit-ID: b39898cd4077f4b6ec706e717c938751c34e1dc4
Gitweb: http://git.kernel.org/tip/b39898cd4077f4b6ec706e717c938751c34e1dc4
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 6 Nov 2013 12:30:07 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 13 Nov 2013 16:03:02 +0100
genirq: Prevent spurious detection for unconditionally polled interrupts
On a 68k platform a couple of interrupts are demultiplexed and
"polled" from a top level interrupt. Unfortunately there is no way to
determine which of the sub interrupts raised the top level interrupt,
so all of the demultiplexed interrupt handlers need to be
invoked. Given a high enough frequency this can trigger the spurious
interrupt detection mechanism, if one of the demultiplex interrupts
returns IRQ_NONE continuously. But this is a false positive as the
polling causes this behaviour and not buggy hardware/software.
Introduce IRQ_POLLED which can be set at interrupt chip setup time via
irq_set_status_flags(). The flag excludes the interrupt from the
spurious detector and from all core polling activities.
Reported-and-tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: linux-m68k@vger.kernel.org
Link: http://lkml.kernel.org/r/alpine.DEB.2.02.1311061149250.23353@ionos.tec.linutronix.de
---
include/linux/irq.h | 7 ++++++-
kernel/irq/settings.h | 7 +++++++
kernel/irq/spurious.c | 12 +++++++++---
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 56bb0dc..7dc1003 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -70,6 +70,9 @@ typedef void (*irq_preflow_handler_t)(struct irq_data *data);
* IRQ_MOVE_PCNTXT - Interrupt can be migrated from process context
* IRQ_NESTED_TRHEAD - Interrupt nests into another thread
* IRQ_PER_CPU_DEVID - Dev_id is a per-cpu variable
+ * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
+ * it from the spurious interrupt detection
+ * mechanism and from core side polling.
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -94,12 +97,14 @@ enum {
IRQ_NESTED_THREAD = (1 << 15),
IRQ_NOTHREAD = (1 << 16),
IRQ_PER_CPU_DEVID = (1 << 17),
+ IRQ_IS_POLLED = (1 << 18),
};
#define IRQF_MODIFY_MASK \
(IRQ_TYPE_SENSE_MASK | IRQ_NOPROBE | IRQ_NOREQUEST | \
IRQ_NOAUTOEN | IRQ_MOVE_PCNTXT | IRQ_LEVEL | IRQ_NO_BALANCING | \
- IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID)
+ IRQ_PER_CPU | IRQ_NESTED_THREAD | IRQ_NOTHREAD | IRQ_PER_CPU_DEVID | \
+ IRQ_IS_POLLED)
#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 1162f10..3320b84 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -14,6 +14,7 @@ enum {
_IRQ_NO_BALANCING = IRQ_NO_BALANCING,
_IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
_IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
+ _IRQ_IS_POLLED = IRQ_IS_POLLED,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};
@@ -26,6 +27,7 @@ enum {
#define IRQ_NOAUTOEN GOT_YOU_MORON
#define IRQ_NESTED_THREAD GOT_YOU_MORON
#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
+#define IRQ_IS_POLLED GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON
@@ -147,3 +149,8 @@ static inline bool irq_settings_is_nested_thread(struct irq_desc *desc)
{
return desc->status_use_accessors & _IRQ_NESTED_THREAD;
}
+
+static inline bool irq_settings_is_polled(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_IS_POLLED;
+}
diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 7b5f012..a1d8cc6 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -67,8 +67,13 @@ static int try_one_irq(int irq, struct irq_desc *desc, bool force)
raw_spin_lock(&desc->lock);
- /* PER_CPU and nested thread interrupts are never polled */
- if (irq_settings_is_per_cpu(desc) || irq_settings_is_nested_thread(desc))
+ /*
+ * PER_CPU, nested thread interrupts and interrupts explicitely
+ * marked polled are excluded from polling.
+ */
+ if (irq_settings_is_per_cpu(desc) ||
+ irq_settings_is_nested_thread(desc) ||
+ irq_settings_is_polled(desc))
goto out;
/*
@@ -268,7 +273,8 @@ try_misrouted_irq(unsigned int irq, struct irq_desc *desc,
void note_interrupt(unsigned int irq, struct irq_desc *desc,
irqreturn_t action_ret)
{
- if (desc->istate & IRQS_POLL_INPROGRESS)
+ if (desc->istate & IRQS_POLL_INPROGRESS ||
+ irq_settings_is_polled(desc))
return;
/* we get here again via the threaded handler */
^ permalink raw reply related [flat|nested] 8+ messages in thread