Re: [PATCH net-next] net: mana: Add support to process throttled EQEs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mana: RDMA queue stalls due to dropped throttle events
--

commit 49b4827fa426b6ae0b8759a5984d01f44b8db5e1
Author: Sahil Chandna <sahilchandna@linux.microsoft.com>

net: mana: Add support to process throttled EQEs

This commit adds support for processing throttled EQEs of type 4 when an
event queue nears full capacity. The driver advertises the
GDMA_DRV_CAP_FLAG_1_THROTTLED_EVENT_QUEUE capability and attempts to schedule
the CQs associated with a throttled EQE. CQs are tracked in a new child_cq
array initialized when creating a CQ.

> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index ae0ed700b3b93..b8e5ad2c6d312 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -747,6 +747,18 @@ void mana_gd_free_service_wq(struct gdma_context *gc)
>  	gc->service_wq = NULL;
>  }
>  
> +static void mana_gd_schedule_eq_cqs(struct gdma_queue *eq)
> +{
> +	struct gdma_queue *cq;
> +	u8 i;
> +
> +	for (i = 0; i < GDMA_EQ_MAX_CHILD_CQ; i++) {
> +		cq = rcu_dereference(eq->eq.child_cq[i]);
> +		if (cq && cq->cq.callback)
> +			cq->cq.callback(cq->cq.context, cq);

[Severity: High]
When a GDMA_EQE_THROTTLE event is processed, this function attempts to notify
the CQs. Will RDMA CQs receive these notifications?

RDMA CQs do not appear to be added to child_cq, which might cause their
notifications to be entirely dropped and result in RDMA queue stalls.

> +	}
> +}
> +

[ ... ]

> @@ -1063,17 +1080,41 @@ static void mana_gd_create_cq(const struct gdma_queue_spec *spec,
>  			      struct gdma_queue *queue)
>  {
>  	u32 log2_num_entries = ilog2(spec->queue_size / GDMA_CQE_SIZE);
> +	struct gdma_queue *parent;
> +	u8 i;
>  
>  	queue->head |= INITIALIZED_OWNER_BIT(log2_num_entries);
> -	queue->cq.parent = spec->cq.parent_eq;
> +	parent = spec->cq.parent_eq;
> +	queue->cq.parent = parent;
>  	queue->cq.context = spec->cq.context;
>  	queue->cq.callback = spec->cq.callback;
> +
> +	if (!parent)
> +		return;
> +
> +	/* For throttled EQE store the child CQ */
> +	for (i = 0; i < GDMA_EQ_MAX_CHILD_CQ; i++)
> +		if (!rcu_access_pointer(parent->eq.child_cq[i])) {
> +			rcu_assign_pointer(parent->eq.child_cq[i], queue);
> +			return;
> +		}

[Severity: High]
Does this logic handle RDMA CQs correctly?

RDMA CQs are created with a NULL parent_eq (via mana_ib_create_kernel_queue),
which causes this function to return early without populating the child_cq
array.

Additionally, if an EQ has more than GDMA_EQ_MAX_CHILD_CQ (which is 2) CQs
attached, which is common in RDMA workloads, this loop will silently return
without adding the additional CQs. Can this lead to lost completions?

>  }

[ ... ]

> diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> index c610fc1067e0d..03fe9395604d6 100644
> --- a/include/net/mana/gdma.h
> +++ b/include/net/mana/gdma.h
> @@ -776,7 +785,8 @@ enum {
>  	 GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY | \
>  	 GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT | \
>  	 GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION | \
> -	 GDMA_DRV_CAP_FLAG_1_NON_CONTIGUOUS_BUFFERS)
> +	 GDMA_DRV_CAP_FLAG_1_NON_CONTIGUOUS_BUFFERS | \
> +	 GDMA_DRV_CAP_FLAG_1_THROTTLED_EVENT_QUEUE)

[Severity: High]
Since this patch globally advertises throttling support, the hardware might
send throttle events on RDMA EQs when they are heavily loaded.

Given that RDMA EQs are initialized with a log2_throttle_limit in
mana_ib_create_eqs(), will enabling this capability cause RDMA workloads to
stall if their throttle events are dropped due to the missing child_cq mapping
in mana_gd_create_cq()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008083315.306076-1-sahilchandna@linux.microsoft.com?part=1