* Re: arm/arm64: GICv2 driver does not have irq_disable implemented
@ 2015-10-09 21:52 ` Thomas Gleixner
0 siblings, 0 replies; 11+ messages in thread
From: Thomas Gleixner @ 2015-10-09 21:52 UTC (permalink / raw)
To: Duc Dang
Cc: Marc Zyngier, Jason Cooper, linux-arm, Linux Kernel Mailing List,
patches
On Fri, 9 Oct 2015, Duc Dang wrote:
> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 9 Oct 2015, Duc Dang wrote:
> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to
> >> disable interrupt before calling __napi_schedule to schedule packet handler
> >> to process the Tx/Rx packets.
> >
> > Which is wrong to begin with. Disable the interrupt at the device
> > level not at the interrupt line level.
> >
> We could not disable the interrupt at Enet controller level due to the
> controller limitation. As you said, using disable_irq_nosync is wrong
> but it looks like that the only option that we have.
Oh well.
> Do you have any suggestion about different approach that we could try?
Try the patch below and add
irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
to your driver before requesting the interrupt. Unset it when freeing
the interrupt.
Thanks,
tglx
8<--------------
Subject: genirq: Add flag to force mask in disable_irq[_nosync]()
From: Thomas Gleixner <tglx@linutronix.de>
Date: Fri, 09 Oct 2015 23:28:58 +0200
If an irq chip does not implement the irq_disable callback, then we
use a lazy approach for disabling the interrupt. That means that the
interrupt is marked disabled, but the interrupt line is not
immediately masked in the interrupt chip. It only becomes masked if
the interrupt is raised while it's marked disabled. We use this to avoid
possibly expensive mask/unmask operations for common case operations.
Unfortunately there are devices which do not allow the interrupt to be
disabled easily at the device level. They are forced to use
disable_irq_nosync(). This can result in taking each interrupt twice.
Instead of enforcing the non lazy mode on all interrupts of a irq
chip, provide a settings flag, which can be set by the driver for that
particular interrupt line.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/irq.h | 4 +++-
kernel/irq/chip.c | 9 +++++++++
kernel/irq/settings.h | 7 +++++++
3 files changed, 19 insertions(+), 1 deletion(-)
Index: tip/include/linux/irq.h
===================================================================
--- tip.orig/include/linux/irq.h
+++ tip/include/linux/irq.h
@@ -72,6 +72,7 @@ enum irqchip_irq_state;
* IRQ_IS_POLLED - Always polled by another interrupt. Exclude
* it from the spurious interrupt detection
* mechanism and from core side polling.
+ * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -97,13 +98,14 @@ enum {
IRQ_NOTHREAD = (1 << 16),
IRQ_PER_CPU_DEVID = (1 << 17),
IRQ_IS_POLLED = (1 << 18),
+ IRQ_DISABLE_UNLAZY = (1 << 19),
};
#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_IS_POLLED)
+ IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
Index: tip/kernel/irq/chip.c
===================================================================
--- tip.orig/kernel/irq/chip.c
+++ tip/kernel/irq/chip.c
@@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
* disabled. If an interrupt happens, then the interrupt flow
* handler masks the line at the hardware level and marks it
* pending.
+ *
+ * If the interrupt chip does not implement the irq_disable callback,
+ * a driver can disable the lazy approach for a particular irq line by
+ * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be
+ * used for devices which cannot disable the interrupt at the device
+ * level under certain circumstances and have to use
+ * disable_irq[_nosync] instead.
*/
void irq_disable(struct irq_desc *desc)
{
@@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
if (desc->irq_data.chip->irq_disable) {
desc->irq_data.chip->irq_disable(&desc->irq_data);
irq_state_set_masked(desc);
+ } else if (irq_settings_disable_unlazy(desc)) {
+ mask_irq(desc);
}
}
Index: tip/kernel/irq/settings.h
===================================================================
--- tip.orig/kernel/irq/settings.h
+++ tip/kernel/irq/settings.h
@@ -15,6 +15,7 @@ enum {
_IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
_IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
_IRQ_IS_POLLED = IRQ_IS_POLLED,
+ _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};
@@ -28,6 +29,7 @@ enum {
#define IRQ_NESTED_THREAD GOT_YOU_MORON
#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
#define IRQ_IS_POLLED GOT_YOU_MORON
+#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON
@@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle
{
return desc->status_use_accessors & _IRQ_IS_POLLED;
}
+
+static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
+}
^ permalink raw reply [flat|nested] 11+ messages in thread* arm/arm64: GICv2 driver does not have irq_disable implemented
2015-10-09 21:52 ` Thomas Gleixner
@ 2015-10-09 22:21 ` Duc Dang
-1 siblings, 0 replies; 11+ messages in thread
From: Duc Dang @ 2015-10-09 22:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 9 Oct 2015, Duc Dang wrote:
>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Fri, 9 Oct 2015, Duc Dang wrote:
>> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to
>> >> disable interrupt before calling __napi_schedule to schedule packet handler
>> >> to process the Tx/Rx packets.
>> >
>> > Which is wrong to begin with. Disable the interrupt at the device
>> > level not at the interrupt line level.
>> >
>> We could not disable the interrupt at Enet controller level due to the
>> controller limitation. As you said, using disable_irq_nosync is wrong
>> but it looks like that the only option that we have.
>
> Oh well.
>
>> Do you have any suggestion about different approach that we could try?
>
> Try the patch below and add
>
> irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
>
> to your driver before requesting the interrupt. Unset it when freeing
> the interrupt.
Thanks, Thomas!
We will try and let you know soon.
>
> Thanks,
>
> tglx
>
> 8<--------------
>
> Subject: genirq: Add flag to force mask in disable_irq[_nosync]()
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Fri, 09 Oct 2015 23:28:58 +0200
>
> If an irq chip does not implement the irq_disable callback, then we
> use a lazy approach for disabling the interrupt. That means that the
> interrupt is marked disabled, but the interrupt line is not
> immediately masked in the interrupt chip. It only becomes masked if
> the interrupt is raised while it's marked disabled. We use this to avoid
> possibly expensive mask/unmask operations for common case operations.
>
> Unfortunately there are devices which do not allow the interrupt to be
> disabled easily at the device level. They are forced to use
> disable_irq_nosync(). This can result in taking each interrupt twice.
>
> Instead of enforcing the non lazy mode on all interrupts of a irq
> chip, provide a settings flag, which can be set by the driver for that
> particular interrupt line.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> include/linux/irq.h | 4 +++-
> kernel/irq/chip.c | 9 +++++++++
> kernel/irq/settings.h | 7 +++++++
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> Index: tip/include/linux/irq.h
> ===================================================================
> --- tip.orig/include/linux/irq.h
> +++ tip/include/linux/irq.h
> @@ -72,6 +72,7 @@ enum irqchip_irq_state;
> * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
> * it from the spurious interrupt detection
> * mechanism and from core side polling.
> + * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -97,13 +98,14 @@ enum {
> IRQ_NOTHREAD = (1 << 16),
> IRQ_PER_CPU_DEVID = (1 << 17),
> IRQ_IS_POLLED = (1 << 18),
> + IRQ_DISABLE_UNLAZY = (1 << 19),
> };
>
> #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_IS_POLLED)
> + IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> Index: tip/kernel/irq/chip.c
> ===================================================================
> --- tip.orig/kernel/irq/chip.c
> +++ tip/kernel/irq/chip.c
> @@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
> * disabled. If an interrupt happens, then the interrupt flow
> * handler masks the line at the hardware level and marks it
> * pending.
> + *
> + * If the interrupt chip does not implement the irq_disable callback,
> + * a driver can disable the lazy approach for a particular irq line by
> + * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be
> + * used for devices which cannot disable the interrupt at the device
> + * level under certain circumstances and have to use
> + * disable_irq[_nosync] instead.
> */
> void irq_disable(struct irq_desc *desc)
> {
> @@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
> if (desc->irq_data.chip->irq_disable) {
> desc->irq_data.chip->irq_disable(&desc->irq_data);
> irq_state_set_masked(desc);
> + } else if (irq_settings_disable_unlazy(desc)) {
> + mask_irq(desc);
> }
> }
>
> Index: tip/kernel/irq/settings.h
> ===================================================================
> --- tip.orig/kernel/irq/settings.h
> +++ tip/kernel/irq/settings.h
> @@ -15,6 +15,7 @@ enum {
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> _IRQ_IS_POLLED = IRQ_IS_POLLED,
> + _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -28,6 +29,7 @@ enum {
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> #define IRQ_IS_POLLED GOT_YOU_MORON
> +#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle
> {
> return desc->status_use_accessors & _IRQ_IS_POLLED;
> }
> +
> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
> +}
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: arm/arm64: GICv2 driver does not have irq_disable implemented
@ 2015-10-09 22:21 ` Duc Dang
0 siblings, 0 replies; 11+ messages in thread
From: Duc Dang @ 2015-10-09 22:21 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Marc Zyngier, Jason Cooper, linux-arm, Linux Kernel Mailing List,
patches
On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 9 Oct 2015, Duc Dang wrote:
>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Fri, 9 Oct 2015, Duc Dang wrote:
>> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to
>> >> disable interrupt before calling __napi_schedule to schedule packet handler
>> >> to process the Tx/Rx packets.
>> >
>> > Which is wrong to begin with. Disable the interrupt at the device
>> > level not at the interrupt line level.
>> >
>> We could not disable the interrupt at Enet controller level due to the
>> controller limitation. As you said, using disable_irq_nosync is wrong
>> but it looks like that the only option that we have.
>
> Oh well.
>
>> Do you have any suggestion about different approach that we could try?
>
> Try the patch below and add
>
> irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
>
> to your driver before requesting the interrupt. Unset it when freeing
> the interrupt.
Thanks, Thomas!
We will try and let you know soon.
>
> Thanks,
>
> tglx
>
> 8<--------------
>
> Subject: genirq: Add flag to force mask in disable_irq[_nosync]()
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Fri, 09 Oct 2015 23:28:58 +0200
>
> If an irq chip does not implement the irq_disable callback, then we
> use a lazy approach for disabling the interrupt. That means that the
> interrupt is marked disabled, but the interrupt line is not
> immediately masked in the interrupt chip. It only becomes masked if
> the interrupt is raised while it's marked disabled. We use this to avoid
> possibly expensive mask/unmask operations for common case operations.
>
> Unfortunately there are devices which do not allow the interrupt to be
> disabled easily at the device level. They are forced to use
> disable_irq_nosync(). This can result in taking each interrupt twice.
>
> Instead of enforcing the non lazy mode on all interrupts of a irq
> chip, provide a settings flag, which can be set by the driver for that
> particular interrupt line.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> include/linux/irq.h | 4 +++-
> kernel/irq/chip.c | 9 +++++++++
> kernel/irq/settings.h | 7 +++++++
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> Index: tip/include/linux/irq.h
> ===================================================================
> --- tip.orig/include/linux/irq.h
> +++ tip/include/linux/irq.h
> @@ -72,6 +72,7 @@ enum irqchip_irq_state;
> * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
> * it from the spurious interrupt detection
> * mechanism and from core side polling.
> + * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
> */
> enum {
> IRQ_TYPE_NONE = 0x00000000,
> @@ -97,13 +98,14 @@ enum {
> IRQ_NOTHREAD = (1 << 16),
> IRQ_PER_CPU_DEVID = (1 << 17),
> IRQ_IS_POLLED = (1 << 18),
> + IRQ_DISABLE_UNLAZY = (1 << 19),
> };
>
> #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_IS_POLLED)
> + IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
>
> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>
> Index: tip/kernel/irq/chip.c
> ===================================================================
> --- tip.orig/kernel/irq/chip.c
> +++ tip/kernel/irq/chip.c
> @@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
> * disabled. If an interrupt happens, then the interrupt flow
> * handler masks the line at the hardware level and marks it
> * pending.
> + *
> + * If the interrupt chip does not implement the irq_disable callback,
> + * a driver can disable the lazy approach for a particular irq line by
> + * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be
> + * used for devices which cannot disable the interrupt at the device
> + * level under certain circumstances and have to use
> + * disable_irq[_nosync] instead.
> */
> void irq_disable(struct irq_desc *desc)
> {
> @@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
> if (desc->irq_data.chip->irq_disable) {
> desc->irq_data.chip->irq_disable(&desc->irq_data);
> irq_state_set_masked(desc);
> + } else if (irq_settings_disable_unlazy(desc)) {
> + mask_irq(desc);
> }
> }
>
> Index: tip/kernel/irq/settings.h
> ===================================================================
> --- tip.orig/kernel/irq/settings.h
> +++ tip/kernel/irq/settings.h
> @@ -15,6 +15,7 @@ enum {
> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
> _IRQ_IS_POLLED = IRQ_IS_POLLED,
> + _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
> };
>
> @@ -28,6 +29,7 @@ enum {
> #define IRQ_NESTED_THREAD GOT_YOU_MORON
> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
> #define IRQ_IS_POLLED GOT_YOU_MORON
> +#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
> #undef IRQF_MODIFY_MASK
> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>
> @@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle
> {
> return desc->status_use_accessors & _IRQ_IS_POLLED;
> }
> +
> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
> +{
> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
> +}
Regards,
Duc Dang.
^ permalink raw reply [flat|nested] 11+ messages in thread* arm/arm64: GICv2 driver does not have irq_disable implemented
2015-10-09 22:21 ` Duc Dang
@ 2015-10-09 23:35 ` Duc Dang
-1 siblings, 0 replies; 11+ messages in thread
From: Duc Dang @ 2015-10-09 23:35 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Oct 9, 2015 at 3:21 PM, Duc Dang <dhdang@apm.com> wrote:
> On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Fri, 9 Oct 2015, Duc Dang wrote:
>>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> > On Fri, 9 Oct 2015, Duc Dang wrote:
>>> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to
>>> >> disable interrupt before calling __napi_schedule to schedule packet handler
>>> >> to process the Tx/Rx packets.
>>> >
>>> > Which is wrong to begin with. Disable the interrupt at the device
>>> > level not at the interrupt line level.
>>> >
>>> We could not disable the interrupt at Enet controller level due to the
>>> controller limitation. As you said, using disable_irq_nosync is wrong
>>> but it looks like that the only option that we have.
>>
>> Oh well.
>>
>>> Do you have any suggestion about different approach that we could try?
>>
>> Try the patch below and add
>>
>> irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
>>
>> to your driver before requesting the interrupt. Unset it when freeing
>> the interrupt.
>
> Thanks, Thomas!
>
> We will try and let you know soon.
Hi Thomas,
We use irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY) in our X-Gene
Ethernet driver along with your patch and interrupt count works as
expected.
Are you going to commit your patch soon?
We will post the patch for our X-Gene Ethernet driver after your patch
is available.
Thanks,
>
>>
>> Thanks,
>>
>> tglx
>>
>> 8<--------------
>>
>> Subject: genirq: Add flag to force mask in disable_irq[_nosync]()
>> From: Thomas Gleixner <tglx@linutronix.de>
>> Date: Fri, 09 Oct 2015 23:28:58 +0200
>>
>> If an irq chip does not implement the irq_disable callback, then we
>> use a lazy approach for disabling the interrupt. That means that the
>> interrupt is marked disabled, but the interrupt line is not
>> immediately masked in the interrupt chip. It only becomes masked if
>> the interrupt is raised while it's marked disabled. We use this to avoid
>> possibly expensive mask/unmask operations for common case operations.
>>
>> Unfortunately there are devices which do not allow the interrupt to be
>> disabled easily at the device level. They are forced to use
>> disable_irq_nosync(). This can result in taking each interrupt twice.
>>
>> Instead of enforcing the non lazy mode on all interrupts of a irq
>> chip, provide a settings flag, which can be set by the driver for that
>> particular interrupt line.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> include/linux/irq.h | 4 +++-
>> kernel/irq/chip.c | 9 +++++++++
>> kernel/irq/settings.h | 7 +++++++
>> 3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> Index: tip/include/linux/irq.h
>> ===================================================================
>> --- tip.orig/include/linux/irq.h
>> +++ tip/include/linux/irq.h
>> @@ -72,6 +72,7 @@ enum irqchip_irq_state;
>> * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
>> * it from the spurious interrupt detection
>> * mechanism and from core side polling.
>> + * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
>> */
>> enum {
>> IRQ_TYPE_NONE = 0x00000000,
>> @@ -97,13 +98,14 @@ enum {
>> IRQ_NOTHREAD = (1 << 16),
>> IRQ_PER_CPU_DEVID = (1 << 17),
>> IRQ_IS_POLLED = (1 << 18),
>> + IRQ_DISABLE_UNLAZY = (1 << 19),
>> };
>>
>> #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_IS_POLLED)
>> + IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
>>
>> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>>
>> Index: tip/kernel/irq/chip.c
>> ===================================================================
>> --- tip.orig/kernel/irq/chip.c
>> +++ tip/kernel/irq/chip.c
>> @@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
>> * disabled. If an interrupt happens, then the interrupt flow
>> * handler masks the line at the hardware level and marks it
>> * pending.
>> + *
>> + * If the interrupt chip does not implement the irq_disable callback,
>> + * a driver can disable the lazy approach for a particular irq line by
>> + * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be
>> + * used for devices which cannot disable the interrupt at the device
>> + * level under certain circumstances and have to use
>> + * disable_irq[_nosync] instead.
>> */
>> void irq_disable(struct irq_desc *desc)
>> {
>> @@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
>> if (desc->irq_data.chip->irq_disable) {
>> desc->irq_data.chip->irq_disable(&desc->irq_data);
>> irq_state_set_masked(desc);
>> + } else if (irq_settings_disable_unlazy(desc)) {
>> + mask_irq(desc);
>> }
>> }
>>
>> Index: tip/kernel/irq/settings.h
>> ===================================================================
>> --- tip.orig/kernel/irq/settings.h
>> +++ tip/kernel/irq/settings.h
>> @@ -15,6 +15,7 @@ enum {
>> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
>> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
>> _IRQ_IS_POLLED = IRQ_IS_POLLED,
>> + _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
>> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
>> };
>>
>> @@ -28,6 +29,7 @@ enum {
>> #define IRQ_NESTED_THREAD GOT_YOU_MORON
>> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
>> #define IRQ_IS_POLLED GOT_YOU_MORON
>> +#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
>> #undef IRQF_MODIFY_MASK
>> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>>
>> @@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle
>> {
>> return desc->status_use_accessors & _IRQ_IS_POLLED;
>> }
>> +
>> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
>> +{
>> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
>> +}
>
> Regards,
> Duc Dang.
Duc Dang.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: arm/arm64: GICv2 driver does not have irq_disable implemented
@ 2015-10-09 23:35 ` Duc Dang
0 siblings, 0 replies; 11+ messages in thread
From: Duc Dang @ 2015-10-09 23:35 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Marc Zyngier, Jason Cooper, linux-arm, Linux Kernel Mailing List,
patches
On Fri, Oct 9, 2015 at 3:21 PM, Duc Dang <dhdang@apm.com> wrote:
> On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Fri, 9 Oct 2015, Duc Dang wrote:
>>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> > On Fri, 9 Oct 2015, Duc Dang wrote:
>>> >> In APM ARM64 X-Gene Enet controller driver, we use disable_irq_nosync to
>>> >> disable interrupt before calling __napi_schedule to schedule packet handler
>>> >> to process the Tx/Rx packets.
>>> >
>>> > Which is wrong to begin with. Disable the interrupt at the device
>>> > level not at the interrupt line level.
>>> >
>>> We could not disable the interrupt at Enet controller level due to the
>>> controller limitation. As you said, using disable_irq_nosync is wrong
>>> but it looks like that the only option that we have.
>>
>> Oh well.
>>
>>> Do you have any suggestion about different approach that we could try?
>>
>> Try the patch below and add
>>
>> irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY);
>>
>> to your driver before requesting the interrupt. Unset it when freeing
>> the interrupt.
>
> Thanks, Thomas!
>
> We will try and let you know soon.
Hi Thomas,
We use irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY) in our X-Gene
Ethernet driver along with your patch and interrupt count works as
expected.
Are you going to commit your patch soon?
We will post the patch for our X-Gene Ethernet driver after your patch
is available.
Thanks,
>
>>
>> Thanks,
>>
>> tglx
>>
>> 8<--------------
>>
>> Subject: genirq: Add flag to force mask in disable_irq[_nosync]()
>> From: Thomas Gleixner <tglx@linutronix.de>
>> Date: Fri, 09 Oct 2015 23:28:58 +0200
>>
>> If an irq chip does not implement the irq_disable callback, then we
>> use a lazy approach for disabling the interrupt. That means that the
>> interrupt is marked disabled, but the interrupt line is not
>> immediately masked in the interrupt chip. It only becomes masked if
>> the interrupt is raised while it's marked disabled. We use this to avoid
>> possibly expensive mask/unmask operations for common case operations.
>>
>> Unfortunately there are devices which do not allow the interrupt to be
>> disabled easily at the device level. They are forced to use
>> disable_irq_nosync(). This can result in taking each interrupt twice.
>>
>> Instead of enforcing the non lazy mode on all interrupts of a irq
>> chip, provide a settings flag, which can be set by the driver for that
>> particular interrupt line.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> include/linux/irq.h | 4 +++-
>> kernel/irq/chip.c | 9 +++++++++
>> kernel/irq/settings.h | 7 +++++++
>> 3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> Index: tip/include/linux/irq.h
>> ===================================================================
>> --- tip.orig/include/linux/irq.h
>> +++ tip/include/linux/irq.h
>> @@ -72,6 +72,7 @@ enum irqchip_irq_state;
>> * IRQ_IS_POLLED - Always polled by another interrupt. Exclude
>> * it from the spurious interrupt detection
>> * mechanism and from core side polling.
>> + * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
>> */
>> enum {
>> IRQ_TYPE_NONE = 0x00000000,
>> @@ -97,13 +98,14 @@ enum {
>> IRQ_NOTHREAD = (1 << 16),
>> IRQ_PER_CPU_DEVID = (1 << 17),
>> IRQ_IS_POLLED = (1 << 18),
>> + IRQ_DISABLE_UNLAZY = (1 << 19),
>> };
>>
>> #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_IS_POLLED)
>> + IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
>>
>> #define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
>>
>> Index: tip/kernel/irq/chip.c
>> ===================================================================
>> --- tip.orig/kernel/irq/chip.c
>> +++ tip/kernel/irq/chip.c
>> @@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
>> * disabled. If an interrupt happens, then the interrupt flow
>> * handler masks the line at the hardware level and marks it
>> * pending.
>> + *
>> + * If the interrupt chip does not implement the irq_disable callback,
>> + * a driver can disable the lazy approach for a particular irq line by
>> + * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can be
>> + * used for devices which cannot disable the interrupt at the device
>> + * level under certain circumstances and have to use
>> + * disable_irq[_nosync] instead.
>> */
>> void irq_disable(struct irq_desc *desc)
>> {
>> @@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
>> if (desc->irq_data.chip->irq_disable) {
>> desc->irq_data.chip->irq_disable(&desc->irq_data);
>> irq_state_set_masked(desc);
>> + } else if (irq_settings_disable_unlazy(desc)) {
>> + mask_irq(desc);
>> }
>> }
>>
>> Index: tip/kernel/irq/settings.h
>> ===================================================================
>> --- tip.orig/kernel/irq/settings.h
>> +++ tip/kernel/irq/settings.h
>> @@ -15,6 +15,7 @@ enum {
>> _IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
>> _IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
>> _IRQ_IS_POLLED = IRQ_IS_POLLED,
>> + _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
>> _IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
>> };
>>
>> @@ -28,6 +29,7 @@ enum {
>> #define IRQ_NESTED_THREAD GOT_YOU_MORON
>> #define IRQ_PER_CPU_DEVID GOT_YOU_MORON
>> #define IRQ_IS_POLLED GOT_YOU_MORON
>> +#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
>> #undef IRQF_MODIFY_MASK
>> #define IRQF_MODIFY_MASK GOT_YOU_MORON
>>
>> @@ -154,3 +156,8 @@ static inline bool irq_settings_is_polle
>> {
>> return desc->status_use_accessors & _IRQ_IS_POLLED;
>> }
>> +
>> +static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
>> +{
>> + return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
>> +}
>
> Regards,
> Duc Dang.
Duc Dang.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip:irq/core] genirq: Add flag to force mask in disable_irq[ _nosync]()
2015-10-09 21:52 ` Thomas Gleixner
(?)
(?)
@ 2015-10-11 9:36 ` tip-bot for Thomas Gleixner
-1 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Thomas Gleixner @ 2015-10-11 9:36 UTC (permalink / raw)
To: linux-tip-commits
Cc: dhdang, mingo, jason, marc.zyngier, linux-kernel, hpa, tglx
Commit-ID: e9849777d0e27cdd2902805be51da73e7c79578c
Gitweb: http://git.kernel.org/tip/e9849777d0e27cdd2902805be51da73e7c79578c
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 9 Oct 2015 23:28:58 +0200
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 11 Oct 2015 11:33:42 +0200
genirq: Add flag to force mask in disable_irq[_nosync]()
If an irq chip does not implement the irq_disable callback, then we
use a lazy approach for disabling the interrupt. That means that the
interrupt is marked disabled, but the interrupt line is not
immediately masked in the interrupt chip. It only becomes masked if
the interrupt is raised while it's marked disabled. We use this to avoid
possibly expensive mask/unmask operations for common case operations.
Unfortunately there are devices which do not allow the interrupt to be
disabled easily at the device level. They are forced to use
disable_irq_nosync(). This can result in taking each interrupt twice.
Instead of enforcing the non lazy mode on all interrupts of a irq
chip, provide a settings flag, which can be set by the driver for that
particular interrupt line.
Reported-and-tested-by: Duc Dang <dhdang@apm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Link: http://lkml.kernel.org/r/alpine.DEB.2.11.1510092348370.6097@nanos
---
include/linux/irq.h | 4 +++-
kernel/irq/chip.c | 9 +++++++++
kernel/irq/manage.c | 1 +
kernel/irq/settings.h | 12 ++++++++++++
4 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index ba72b60..3c1c967 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -72,6 +72,7 @@ enum irqchip_irq_state;
* IRQ_IS_POLLED - Always polled by another interrupt. Exclude
* it from the spurious interrupt detection
* mechanism and from core side polling.
+ * IRQ_DISABLE_UNLAZY - Disable lazy irq disable
*/
enum {
IRQ_TYPE_NONE = 0x00000000,
@@ -97,13 +98,14 @@ enum {
IRQ_NOTHREAD = (1 << 16),
IRQ_PER_CPU_DEVID = (1 << 17),
IRQ_IS_POLLED = (1 << 18),
+ IRQ_DISABLE_UNLAZY = (1 << 19),
};
#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_IS_POLLED)
+ IRQ_IS_POLLED | IRQ_DISABLE_UNLAZY)
#define IRQ_NO_BALANCING_MASK (IRQ_PER_CPU | IRQ_NO_BALANCING)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 4aa00d3..1520645 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -241,6 +241,13 @@ void irq_enable(struct irq_desc *desc)
* disabled. If an interrupt happens, then the interrupt flow
* handler masks the line at the hardware level and marks it
* pending.
+ *
+ * If the interrupt chip does not implement the irq_disable callback,
+ * a driver can disable the lazy approach for a particular irq line by
+ * calling 'irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY)'. This can
+ * be used for devices which cannot disable the interrupt at the
+ * device level under certain circumstances and have to use
+ * disable_irq[_nosync] instead.
*/
void irq_disable(struct irq_desc *desc)
{
@@ -248,6 +255,8 @@ void irq_disable(struct irq_desc *desc)
if (desc->irq_data.chip->irq_disable) {
desc->irq_data.chip->irq_disable(&desc->irq_data);
irq_state_set_masked(desc);
+ } else if (irq_settings_disable_unlazy(desc)) {
+ mask_irq(desc);
}
}
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 312f9cb..a71175f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1463,6 +1463,7 @@ static struct irqaction *__free_irq(unsigned int irq, void *dev_id)
/* If this was the last handler, shut down the IRQ line: */
if (!desc->action) {
+ irq_settings_clr_disable_unlazy(desc);
irq_shutdown(desc);
irq_release_resources(desc);
}
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 3320b84..320579d 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -15,6 +15,7 @@ enum {
_IRQ_NESTED_THREAD = IRQ_NESTED_THREAD,
_IRQ_PER_CPU_DEVID = IRQ_PER_CPU_DEVID,
_IRQ_IS_POLLED = IRQ_IS_POLLED,
+ _IRQ_DISABLE_UNLAZY = IRQ_DISABLE_UNLAZY,
_IRQF_MODIFY_MASK = IRQF_MODIFY_MASK,
};
@@ -28,6 +29,7 @@ enum {
#define IRQ_NESTED_THREAD GOT_YOU_MORON
#define IRQ_PER_CPU_DEVID GOT_YOU_MORON
#define IRQ_IS_POLLED GOT_YOU_MORON
+#define IRQ_DISABLE_UNLAZY GOT_YOU_MORON
#undef IRQF_MODIFY_MASK
#define IRQF_MODIFY_MASK GOT_YOU_MORON
@@ -154,3 +156,13 @@ static inline bool irq_settings_is_polled(struct irq_desc *desc)
{
return desc->status_use_accessors & _IRQ_IS_POLLED;
}
+
+static inline bool irq_settings_disable_unlazy(struct irq_desc *desc)
+{
+ return desc->status_use_accessors & _IRQ_DISABLE_UNLAZY;
+}
+
+static inline void irq_settings_clr_disable_unlazy(struct irq_desc *desc)
+{
+ desc->status_use_accessors &= ~_IRQ_DISABLE_UNLAZY;
+}
^ permalink raw reply related [flat|nested] 11+ messages in thread