netdev - RE: [PATCH net-next v3 5/6] virtio_net: Add a lock for per queue RX coalesce

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]

Date: Tue, 16 Apr 2024 03:15:34 +0000
From: Dan Jurgens <danielj@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "mst@...hat.com"
	<mst@...hat.com>, "jasowang@...hat.com" <jasowang@...hat.com>,
	"xuanzhuo@...ux.alibaba.com" <xuanzhuo@...ux.alibaba.com>,
	"virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "pabeni@...hat.com" <pabeni@...hat.com>, Jiri Pirko
	<jiri@...dia.com>
Subject: RE: [PATCH net-next v3 5/6] virtio_net: Add a lock for per queue RX
 coalesce

> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Friday, April 12, 2024 9:21 PM
> To: Dan Jurgens <danielj@...dia.com>
> Cc: netdev@...r.kernel.org; mst@...hat.com; jasowang@...hat.com;
> xuanzhuo@...ux.alibaba.com; virtualization@...ts.linux.dev;
> davem@...emloft.net; edumazet@...gle.com; pabeni@...hat.com; Jiri
> Pirko <jiri@...dia.com>
> Subject: Re: [PATCH net-next v3 5/6] virtio_net: Add a lock for per queue RX
> coalesce
> 
> On Fri, 12 Apr 2024 14:53:08 -0500 Daniel Jurgens wrote:
> > Once the RTNL locking around the control buffer is removed there can
> > be contention on the per queue RX interrupt coalescing data. Use a
> > spin lock per queue.
> 
> Does not compile on Clang.

Which version? It compiles for me with:
$ clang -v
clang version 15.0.7 (Fedora 15.0.7-2.fc37)

> 
> > +			scoped_guard(spinlock, &vi->rq[i].intr_coal_lock) {
> > +				err = virtnet_send_rx_ctrl_coal_vq_cmd(vi, i,
> > +								       vi-
> >intr_coal_rx.max_usecs,
> > +								       vi-
> >intr_coal_rx.max_packets);
> > +				if (err)
> > +					return err;
> > +			}
> 
> Do you really think this needs a scoped guard and 4th indentation level,
> instead of just:
> 
> 			..lock(..);
> 			err = virtnet_send_rx_ctrl_coal_vq_cmd(vi, i,
> 							       vi-
> >intr_coal_rx.max_usecs,
> 							       vi-
> >intr_coal_rx.max_packets);
> 			..unlock(..);

I'll change it in the next version.

> 			if (err)
> 				return err;
> 
> > +		scoped_guard(spinlock, &vi->rq[i].intr_coal_lock) {
> > +			vi->rq[i].intr_coal.max_usecs = ec-
> >rx_coalesce_usecs;
> > +			vi->rq[i].intr_coal.max_packets = ec-
> >rx_max_coalesced_frames;
> > +		}
> 
> :-|
> --
> pw-bot: cr