Re: [PATCH net 6/8] ice: improve updating ice_{t, r}x_ring::xsk_pool

[Jakub Kicinski <kuba@kernel.org>]

On Mon,  8 Jul 2024 15:14:12 -0700 Tony Nguyen wrote:
> @@ -1556,7 +1556,7 @@ int ice_napi_poll(struct napi_struct *napi, int budget)
>  		 * comparison in the irq context instead of many inside the
>  		 * ice_clean_rx_irq function and makes the codebase cleaner.
>  		 */
> -		cleaned = rx_ring->xsk_pool ?
> +		cleaned = READ_ONCE(rx_ring->xsk_pool) ?
>  			  ice_clean_rx_irq_zc(rx_ring, budget_per_ring) :
>  			  ice_clean_rx_irq(rx_ring, budget_per_ring);
>  		work_done += cleaned;


> @@ -832,8 +839,8 @@ ice_add_xsk_frag(struct ice_rx_ring *rx_ring, struct xdp_buff *first,
>   */
>  int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget)
>  {
> +	struct xsk_buff_pool *xsk_pool = READ_ONCE(rx_ring->xsk_pool);
>  	unsigned int total_rx_bytes = 0, total_rx_packets = 0;
> -	struct xsk_buff_pool *xsk_pool = rx_ring->xsk_pool;
>  	u32 ntc = rx_ring->next_to_clean;
>  	u32 ntu = rx_ring->next_to_use;
>  	struct xdp_buff *first = NULL;

This looks suspicious, you need to at least explain why it's correct.
READ_ONCE() means one access per critical section, usually.
You access it at least twice in a single NAPI pool.