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 <[email protected]> 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/[email protected]?part=1
