Re: [PATCH net-next 4/5] net/mlx4: track whether a TX doorbell is pending

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] net/mlx4: doorbell_pending placed in read-mostly cacheline of
  mlx4_en_tx_ring
--

commit c4a2c0626cb72780e7edec3e4335fdf04dad4de9
Author: Joe Damato <joe@dama.to>

net/mlx4: track whether a TX doorbell is pending

This commit adds a doorbell_pending flag to struct mlx4_en_tx_ring to track
when a TX doorbell write has been deferred due to xmit_more.

> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index aab97694f86b3..601c09020f380 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -281,6 +281,7 @@ struct mlx4_en_tx_ring {
>  	unsigned long		tso_packets;
>  	unsigned long		xmit_more;
>  	struct mlx4_bf		bf;
> +	bool			doorbell_pending;
>  
>  	/* Following part should be mostly read */
>  	void __iomem		*doorbell_address;

[Severity: Low]
Does placing doorbell_pending after struct mlx4_bf put a per-packet written
field into the read-mostly cacheline of struct mlx4_en_tx_ring?

On 64-bit SMP builds, the TX producer cacheline starting at prod (offset
0x40) is filled through bf.uar, while the read-only bf.reg pointer starts
the next cacheline at offset 0x80 alongside doorbell_address, size_mask,
buf, and tx_info, as arranged by commit e3f42f845390 ("mlx4: reorganize
struct mlx4_en_tx_ring").

Because mlx4_en_tx_write_desc() and mlx4_en_xmit_doorbell() write
doorbell_pending on every packet, placing it at offset 0x88 dirties that
read-mostly cacheline on every TX packet, causing false sharing with TX
completion polling in mlx4_en_process_tx_cq().  The 7 bytes of alignment
padding before doorbell_address also push bf_enabled and hwtstamp_tx_type
into a third cacheline.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110231304.1859217-1-joe@dama.to?part=4