Re: [PATCH RFC] r8152: Ensure that napi_schedule() is handled

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, May 14 2021 at 13:46, Jakub Kicinski wrote:
> On Fri, 14 May 2021 22:25:50 +0200 Thomas Gleixner wrote:
>> Except that some instruction cycle beancounters might complain about
>> the extra conditional for the sane cases.
>> 
>> But yes, I'm fine with that as well. That's why this patch is marked RFC :)
>
> When we're in the right context (irq/bh disabled etc.) the cost is just
> read of preempt_count() and jump, right? And presumably preempt_count()
> is in the cache already, because those sections aren't very long. Let me
> make this change locally and see if it is in any way perceivable.

Right. Just wanted to mention it :)

> Obviously if anyone sees a way to solve the problem without much
> ifdefinery and force_irqthreads checks that'd be great - I don't.

This is not related to force_irqthreads at all. This very driver invokes
it from plain thread context.

> I'd rather avoid pushing this kind of stuff out to the drivers.

You could have napi_schedule_intask() or something like that which would
do the local_bh_disable()/enable() dance around the invocation of
napi_schedule(). That would also document it clearly in the drivers. A
quick grep shows a bunch of instances which could be replaced:

drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c-5704-		local_bh_disable();
drivers/net/ethernet/mellanox/mlx4/en_netdev.c-1830-		local_bh_disable();
drivers/net/usb/r8152.c-1552-	local_bh_disable();
drivers/net/virtio_net.c-1355-	local_bh_disable();
drivers/net/wireless/intel/iwlwifi/pcie/rx.c-1650-	local_bh_disable();
drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2015-		local_bh_disable();
drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2225-		local_bh_disable();
drivers/net/wireless/intel/iwlwifi/pcie/rx.c-2235-		local_bh_disable();
drivers/s390/net/qeth_core_main.c-3515-	local_bh_disable();

Thanks,

        tglx