netdev - RE: [EXTERNAL] Re: [PATCH 2/2] net: ks8851: Handle softirqs at the end of IRQ thread to fix hang

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: 
 <MWHPR1801MB191894EAC71A311B0115C69AD33F2@MWHPR1801MB1918.namprd18.prod.outlook.com>
Date: Mon, 1 Apr 2024 14:13:43 +0000
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Marek Vasut <marex@...x.de>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S. Miller"
	<davem@...emloft.net>,
        Uwe Kleine-König
	<u.kleine-koenig@...gutronix.de>,
        Andy Shevchenko
	<andriy.shevchenko@...ux.intel.com>,
        Dmitry Torokhov
	<dmitry.torokhov@...il.com>,
        Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>,
        Mark Brown <broonie@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>,
        Ronald Wahl <ronald.wahl@...itan.com>,
        Simon Horman
	<horms@...nel.org>
Subject: RE: [EXTERNAL] Re: [PATCH 2/2] net: ks8851: Handle softirqs at the
 end of IRQ thread to fix hang

> From: Marek Vasut <marex@...x.de>
> To: Ratheesh Kannoth <rkannoth@...vell.com>
> Cc: netdev@...r.kernel.org; David S. Miller <davem@...emloft.net>; Uwe
> This test here has been taken from net/core/dev.c netif_rx() , it is the same
> one used there around __netif_rx() invocation.
> 
> >>   	struct ks8851_net *ks = _ks;
> >>   	unsigned handled = 0;
> >>   	unsigned long flags;
> >>   	unsigned int status;
> >>
> >> +	if (need_bh_off)
> >> +		local_bh_disable();
> > This threaded irq's thread function (ks8851_irq()) will always run in process
> context, right ?
> 
> I think so.
> 
> > Do you need "if(need_bh_off)" loop?
My bad. Typo. I meant "if (need_bh_off) statement"; not "loop". 

> It is not a loop, it is invoked once. It is here to disable BHs so that the
> net_rx_action BH wouldn't run until after the spinlock protected section of the
> IRQ handler. Te net_rx_action may end up calling ks8851_start_xmit_par,
> which must be called with the spinlock released, otherwise the system would
> lock up.
I understand that. My question - will there be a case (currently, without this patch)  ks8851_irq()
Is called after disabling local BH.  If it is always called without disabled, can we avoid "if" statement.
altogether?