Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler

[Marek Vasut]

On 4/9/26 8:52 AM, Nicolai Buchwitz wrote:

Hello Nicolai,

> > @@ -408,7 +426,9 @@ static int ks8851_net_open(struct net_device *dev)
> >      unsigned long flags;
> >      int ret;
> >
> > -    ret = request_threaded_irq(dev->irq, NULL, ks8851_irq,
> > +    ret = request_threaded_irq(dev->irq, NULL,
> > +                   ks->no_bh_in_irq_handler ?
> > +                   ks8851_irq_nobh : ks8851_irq,
>
> This works, but wouldn't it be simpler to put the BH disable
> into the PAR lock/unlock directly?
>
>    static void ks8851_lock_par(...)
>    {
>        local_bh_disable();
>        spin_lock_irqsave(&ksp->lock, *flags);
>    }
>
>    static void ks8851_unlock_par(...)
>    {
>        spin_unlock_irqrestore(&ksp->lock, *flags);
>        local_bh_enable();
>    }
>
> No flag, no wrapper, no conditional in request_threaded_irq.
> And it protects all PAR lock/unlock callsites, not just the
> IRQ handler.
That is exactly why I wrapped the IRQ handler, because the BH should be 
disabled ONLY around the IRQ handler, not around the other call sites.