linux-kernel - Re: recommended use of request_any_context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADRPPNS4fYPuTq7WfgsLRPo+nrWegU1s6_7k-FWmtJAJ_SCKkw@mail.gmail.com>
Date:   Fri, 23 Sep 2016 13:33:39 -0500
From:   Leo Li <pku.leo@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Marc Zyngier <marc.zyngier@....com>,
        Marc Zyngier <maz@...terjones.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: recommended use of request_any_context_irq()

On Thu, Sep 22, 2016 at 5:03 PM, Thomas Gleixner <tglx@...utronix.de> 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;
> +                       }
>                 }
>         }
>
>
>