linux-kernel - Re: [resend PATCH 2/2] dim: pass dim_sample to net

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: <CADUfDZpBfwGJwhUHCZk8AgZDY0QP3j2dEUHZfC1VkR+75jj2WA@mail.gmail.com>
Date: Sun, 3 Nov 2024 14:50:48 -0800
From: Caleb Sander <csander@...estorage.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, 
	Arthur Kiyanovski <akiyano@...zon.com>, Brett Creeley <brett.creeley@....com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, 
	Christophe Leroy <christophe.leroy@...roup.eu>, Claudiu Manoil <claudiu.manoil@....com>, 
	David Arinzon <darinzon@...zon.com>, "David S. Miller" <davem@...emloft.net>, 
	Doug Berger <opendmb@...il.com>, Eric Dumazet <edumazet@...gle.com>, 
	Eugenio Pérez <eperezma@...hat.com>, 
	Felix Fietkau <nbd@....name>, Florian Fainelli <florian.fainelli@...adcom.com>, 
	Geetha sowjanya <gakula@...vell.com>, hariprasad <hkelam@...vell.com>, 
	Jason Wang <jasowang@...hat.com>, Jonathan Corbet <corbet@....net>, Leon Romanovsky <leon@...nel.org>, 
	Lorenzo Bianconi <lorenzo@...nel.org>, Louis Peens <louis.peens@...igine.com>, 
	Mark Lee <Mark-MC.Lee@...iatek.com>, Matthias Brugger <matthias.bgg@...il.com>, 
	Michael Chan <michael.chan@...adcom.com>, "Michael S. Tsirkin" <mst@...hat.com>, 
	Noam Dagan <ndagan@...zon.com>, Paolo Abeni <pabeni@...hat.com>, 
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, Roy Pledge <Roy.Pledge@....com>, 
	Saeed Bishara <saeedb@...zon.com>, Saeed Mahameed <saeedm@...dia.com>, 
	Sean Wang <sean.wang@...iatek.com>, Shannon Nelson <shannon.nelson@....com>, 
	Shay Agroskin <shayagr@...zon.com>, Simon Horman <horms@...nel.org>, 
	Subbaraya Sundeep <sbhatta@...vell.com>, Sunil Goutham <sgoutham@...vell.com>, 
	Tal Gilboa <talgi@...dia.com>, Tariq Toukan <tariqt@...dia.com>, 
	Tony Nguyen <anthony.l.nguyen@...el.com>, Vladimir Oltean <vladimir.oltean@....com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, intel-wired-lan@...ts.osuosl.org, 
	linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org, 
	linuxppc-dev@...ts.ozlabs.org, linux-rdma@...r.kernel.org, 
	netdev@...r.kernel.org, oss-drivers@...igine.com, 
	virtualization@...ts.linux.dev
Subject: Re: [resend PATCH 2/2] dim: pass dim_sample to net_dim() by reference

On Sun, Nov 3, 2024 at 12:21 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed, 30 Oct 2024 18:23:26 -0600 Caleb Sander Mateos wrote:
> > In a heavy TCP workload, mlx5e_handle_rx_dim() consumes 3% of CPU time,
> > 94% of which is attributed to the first push instruction to copy
> > dim_sample on the stack for the call to net_dim():
>
> Change itself looks fine, so we can apply, but this seems surprising.
> Are you sure this is not just some measurement problem?
> Do you see 3% higher PPS with this change applied?

Agreed, this bottleneck surprised me too. But the CPU profiles clearly
point to this push instruction in mlx5e_handle_rx_dim() being very
hot. My best explanation is that the 2- and 4-byte stores followed
immediately by 8-byte loads from the same addresses cannot be
pipelined effectively. The loads must wait for the stores to complete
before reading back the values they wrote. Ideally the compiler would
recognize that the struct dim_sample local variable is only used to
pass to net_dim() and avoid duplicating it. I guess passing large
structs by value in C is not very common, so there probably isn't as
much effort put into optimizing it.
With the patches applied, the CPU time spent in mlx5e_handle_rx_dim()
(excluding children) drops from 3.14% to 0.08%. Unfortunately, there
are other bottlenecks in the system and 1% variation in the throughput
is typical, so the patches don't translate into a clear 3% increase in
throughput.

Best,
Caleb