Re: arm/arm64: GICv2 driver does not have irq_disable implemented
From: Duc Dang
Date: Fri Oct 09 2015 - 19:35:40 EST
On Fri, Oct 9, 2015 at 3:21 PM, Duc Dang <dhdang@xxxxxxx> wrote:
> On Fri, Oct 9, 2015 at 2:52 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
>> On Fri, 9 Oct 2015, Duc Dang wrote:
>>> On Fri, Oct 9, 2015 at 10:52 AM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/