On Sat, May 14, 2022 at 05:06:52PM +0200, Maxime Chevallier wrote:
> +/* locking is handled by the caller */
> +static int ipqess_rx_buf_alloc_napi(struct ipqess_rx_ring *rx_ring)
> +{
> + struct ipqess_buf *buf = &rx_ring->buf[rx_ring->head];
> +
> + buf->skb = napi_alloc_skb(&rx_ring->napi_rx, IPQESS_RX_HEAD_BUFF_SIZE);
> + if (!buf->skb)
> + return -ENOMEM;
> +
> + return ipqess_rx_buf_prepare(buf, rx_ring);
> +}
> +
> +static int ipqess_rx_buf_alloc(struct ipqess_rx_ring *rx_ring)
> +{
> + struct ipqess_buf *buf = &rx_ring->buf[rx_ring->head];
> +
> + buf->skb = netdev_alloc_skb_ip_align(rx_ring->ess->netdev,
> + IPQESS_RX_HEAD_BUFF_SIZE);
> +
> + if (!buf->skb)
> + return -ENOMEM;
> +
> + return ipqess_rx_buf_prepare(buf, rx_ring);
> +}
> +
> +static void ipqess_refill_work(struct work_struct *work)
> +{
> + struct ipqess_rx_ring_refill *rx_refill = container_of(work,
> + struct ipqess_rx_ring_refill, refill_work);
> + struct ipqess_rx_ring *rx_ring = rx_refill->rx_ring;
> + int refill = 0;
> +
> + /* don't let this loop by accident. */
> + while (atomic_dec_and_test(&rx_ring->refill_count)) {
> + napi_disable(&rx_ring->napi_rx);
> + if (ipqess_rx_buf_alloc(rx_ring)) {
> + refill++;
> + dev_dbg(rx_ring->ppdev,
> + "Not all buffers were reallocated");
> + }
> + napi_enable(&rx_ring->napi_rx);
> + }
> +
> + if (atomic_add_return(refill, &rx_ring->refill_count))
> + schedule_work(&rx_refill->refill_work);
> +}
> +
> +static int ipqess_rx_poll(struct ipqess_rx_ring *rx_ring, int budget)
> +{
> + while (done < budget) {
> + num_desc += atomic_xchg(&rx_ring->refill_count, 0);
> + while (num_desc) {
> + if (ipqess_rx_buf_alloc_napi(rx_ring)) {
> + num_desc = atomic_add_return(num_desc,
> + &rx_ring->refill_count);
> + if (num_desc >= ((4 * IPQESS_RX_RING_SIZE + 6) / 7))
DIV_ROUND_UP(IPQESS_RX_RING_SIZE * 4, 7)
Also, why this number?
> + schedule_work(&rx_ring->ess->rx_refill[rx_ring->ring_id].refill_work);
> + break;
> + }
> + num_desc--;
> + }
> + }
> +
> + ipqess_w32(rx_ring->ess, IPQESS_REG_RX_SW_CONS_IDX_Q(rx_ring->idx),
> + rx_ring_tail);
> + rx_ring->tail = rx_ring_tail;
> +
> + return done;
> +}
> +static void ipqess_rx_ring_free(struct ipqess *ess)
> +{
> + int i;
> +
> + for (i = 0; i < IPQESS_NETDEV_QUEUES; i++) {
> + int j;
> +
> + atomic_set(&ess->rx_ring[i].refill_count, 0);
> + cancel_work_sync(&ess->rx_refill[i].refill_work);
When refill_work is currently scheduled and executing the while loop,
will refill_count underflow due to the possibility of calling
atomic_dec_and_test(0)?
> +
> + for (j = 0; j < IPQESS_RX_RING_SIZE; j++) {
> + dma_unmap_single(&ess->pdev->dev,
> + ess->rx_ring[i].buf[j].dma,
> + ess->rx_ring[i].buf[j].length,
> + DMA_FROM_DEVICE);
> + dev_kfree_skb_any(ess->rx_ring[i].buf[j].skb);
> + }
> + }
> +}
