linux-kernel - Re: [Softirq] a76eadba0d: WARNING:at_net/mac80211/rx.c:#ieee80211_rx

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKc596Lp3m3Ny9joOckYPsR5Lmok978bceMYNBrzsk6ozFjqhw@mail.gmail.com>
Date:   Fri, 24 Jul 2020 13:35:17 +0800
From:   jun qian <qianjun.kernel@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     kernel test robot <lkp@...el.com>, peterz@...radead.org,
        will@...nel.org, luto@...nel.org, linux-kernel@...r.kernel.org,
        Uladzislau Rezki <urezki@...il.com>,
        Yafang Shao <laoar.shao@...il.com>, lkp@...ts.01.org
Subject: Re: [Softirq] a76eadba0d: WARNING:at_net/mac80211/rx.c:#ieee80211_rx_napi[mac80211]

On Thu, Jul 23, 2020 at 10:35 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> jun qian <qianjun.kernel@...il.com> writes:
> > On Thu, Jul 23, 2020 at 6:58 PM Thomas Gleixner <tglx@...utronix.de> wrote:
> >> That drops everything which has not yet been processed and the above
> >> warning is due to this.
> >>
> > wow, I made a mistake, thank you for finding the cause of the problem
> > so quickly.
> >
> > How about the following code.   we need to clear the corresponding
> > pending bit at the
> > right time Instead of all the pending bits cleared in the start.
> >
> > pending = softirq_pending();
> >
> > while ((softirq_bit = ffs(pending))) {
> >
> >         pending >>= softirq_bit;
> >
> >         set_softirq_pending(pending);  //Only clear the corresponding
> > bit which will be processed.
>
> How is that supposed to be correct. pending has been shifted
> right. Something like this should work:
>
>                 h++;
>                 pending >>= softirq_bit;
>
>                 if (timeout()) {
>                         /*
>                          * Ensure that the remaining pending bits
>                          * are handled.
>                          */
>                         or_softirq_pending(pending << (vec_nr + 1));
>                         break;
>                 }
>         }
>
> Thanks,
>
>         tglx
>

I have two questions that need to be discussed.

1. If the __do_sofrirq() is executed in the ksoftirqd, we may not need
to check the timeout in the loop.
2. Both the invoke_softirq() and run_ksoftirqd()  will execute
__do_sofirq, they all execute the same code，
    when it is in the ksoftirqd, Do we need to wake up ksoftirqd in
the process context according to
    max_restart and MAX_SOFTIRQ_TIME. In my opinion, If we  use a flag
to distinguish where
    __do_softirq() is called from,  we can do what is most suitable
for __do_softirq based on this flag.