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.
