On Wed, Aug 07, 2024 at 04:17:06PM -0700, longli@xxxxxxxxxxxxxxxxx wrote:
> From: Long Li <longli@xxxxxxxxxxxxx>
>
> After napi_complete_done() is called when NAPI is polling in the current
> process context, another NAPI may be scheduled and start running in
> softirq on another CPU and may ring the doorbell before the current CPU
> does. When combined with unnecessary rings when there is no need to arm
> the CQ, it triggers error paths in the hardware.
>
> This patch fixes this by calling napi_complete_done() after doorbell
> rings. It limits the number of unnecessary rings when there is
> no need to arm. MANA hardware specifies that there must be one doorbell
> ring every 8 CQ wraparounds. This driver guarantees one doorbell ring as
> soon as the number of consumed CQEs exceeds 4 CQ wraparounds. In pratical
> workloads, the 4 CQ wraparounds proves to be big enough that it rarely
> exceeds this limit before all the napi weight is consumed.
>
> To implement this, add a per-CQ counter cq->work_done_since_doorbell,
> and make sure the CQ is armed as soon as passing 4 wraparounds of the CQ.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: e1b5683ff62e ("net: mana: Move NAPI from EQ to CQ")
>
> Reviewed-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> ---
>
> change in v2:
> Added more details to comments to explain the patch
>
> drivers/net/ethernet/microsoft/mana/mana_en.c | 24 ++++++++++++-------
> include/net/mana/mana.h | 1 +
> 2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index d2f07e179e86..f83211f9e737 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1788,7 +1788,6 @@ static void mana_poll_rx_cq(struct mana_cq *cq)
> static int mana_cq_handler(void *context, struct gdma_queue *gdma_queue)
> {
> struct mana_cq *cq = context;
> - u8 arm_bit;
> int w;
>
> WARN_ON_ONCE(cq->gdma_cq != gdma_queue);
> @@ -1799,16 +1798,23 @@ static int mana_cq_handler(void *context, struct gdma_queue *gdma_queue)
> mana_poll_tx_cq(cq);
>
> w = cq->work_done;
> -
> - if (w < cq->budget &&
> - napi_complete_done(&cq->napi, w)) {
> - arm_bit = SET_ARM_BIT;
> - } else {
> - arm_bit = 0;
> + cq->work_done_since_doorbell += w;
> +
> + if (w < cq->budget) {
> + mana_gd_ring_cq(gdma_queue, SET_ARM_BIT);
> + cq->work_done_since_doorbell = 0;
> + napi_complete_done(&cq->napi, w);
> + } else if (cq->work_done_since_doorbell >
> + cq->gdma_cq->queue_size / COMP_ENTRY_SIZE * 4) {
should we define a macro for 4? may be 'CQ_WRAPAROUND_LIMIT'
> + /* MANA hardware requires at least one doorbell ring every 8
> + * wraparounds of CQ even if there is no need to arm the CQ.
> + * This driver rings the doorbell as soon as we have exceeded
> + * 4 wraparounds.
> + */
> + mana_gd_ring_cq(gdma_queue, 0);
> + cq->work_done_since_doorbell = 0;
> }
>
> - mana_gd_ring_cq(gdma_queue, arm_bit);
> -
> return w;
> }
>
> diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> index 6439fd8b437b..7caa334f4888 100644
> --- a/include/net/mana/mana.h
> +++ b/include/net/mana/mana.h
> @@ -275,6 +275,7 @@ struct mana_cq {
> /* NAPI data */
> struct napi_struct napi;
> int work_done;
> + int work_done_since_doorbell;
> int budget;
> };
>
> --
> 2.17.1
