netdev - Re: [PATCH net] bnx2x: Fix netpoll interoperability

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: <1429033912.7346.47.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Tue, 14 Apr 2015 10:51:52 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Yuval Mintz <Yuval.Mintz@...gic.com>
Cc:	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>, "bind@...s.net" <bind@...s.net>,
	"peter@...leysoftware.com" <peter@...leysoftware.com>,
	Ariel Elior <Ariel.Elior@...gic.com>,
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] bnx2x: Fix netpoll interoperability

On Tue, 2015-04-14 at 16:44 +0000, Yuval Mintz wrote:
> >> Commit 9a2620c877454 ("bnx2x: prevent WARN during driver unload") switched
> >> the napi/busy_lock locking mechanism from spin_lock() into spin_lock_bh(),
> >> breaking inter-operability with netconsole, as netpoll disables interrupts
> >> prior to calling our napi mechanism.
> >>
> >> This switches the driver into using atomic assignments instead of the spinlock
> >> mechanisms previously employed.
> 
> > This fix is prone to starvation of softirq handler.
> > Since you use 2 bits, I believe you could set one bit unconditionally to
> > make sure the bnx2x_low_latency_recv() will yield to softirq handler.
> 
> Care to elaborate a bit? About the possibly starvation?
> 
> > Please CC me and Willem on busypoll stuff, thanks !

I believe the original busypoll yield concern was forgotten or reversed.

If you have many busypoll users, they might get the 'lock', and the poor
cpu that received the hardware interrupt and raised RX softirq for this
RX queue will spin forever [1]

busypoll users should backoff and not attempt to grab a lock (especially
if this lock is not a ticket lock anymore) if the softirq handler is
desperately trying to make progress.

[1]
sofirq handler

   while (1) {
       if (!bnx2x_fp_lock_napi())
		continue;
        }
   }

I can provide an alternative patch if you like.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html