Re: recommended use of request_any_context_irq()

From: Leo Li
Date: Fri Sep 23 2016 - 14:33:46 EST


On Thu, Sep 22, 2016 at 5:03 PM, Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> Leo,
>
> On Thu, 22 Sep 2016, Leo Li wrote:
>> core code. And my concerns is that there are other drivers can hit
>> the same problem if connected to the threaded interrupt controller.
>> What can we do prevent similar problem in the future?
>
> The simplest way is to be more informative in the failure case. See patch
> below.
>
> Converting drivers is surely something which can be done, but I wouldn't
> try to do a wholesale conversion blindly.

Thomas,

It is not ideal but definitely helpful to provide more information
when there is an issue.

>
> Thanks,
>
> tglx
>
> 8<----------------
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 73a2b786b5e9..605915f689b4 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1146,6 +1146,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> nested = irq_settings_is_nested_thread(desc);
> if (nested) {
> if (!new->thread_fn) {
> + pr_warn("IRQ%u is nested, but thread_fn is NULL\n", irq);

Can we provide more information in this message? It is still not
clear enough to people who are not familiar with threaded interrupt.

How about "IRQ%u is requesting standard interrupt while connected to
threaded interrupt controller\n Consider using
request_any_context_irq()."?

> ret = -EINVAL;
> goto out_mput;
> }
> @@ -1158,8 +1159,10 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
> } else {
> if (irq_settings_can_thread(desc)) {
> ret = irq_setup_forced_threading(new);
> - if (ret)
> + if (ret) {
> + pr_warn("IRQ%u failed to setup thread\n", irq);
> goto out_mput;
> + }
> }
> }
>
>
>