Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] networking: u64_stats_sync writer in mana_fold_rxq_stats does not disable preemption/BH --
commit 93d72a1c224b35993067fa6f5051dfe42f90cf43 Author: Long Li <[email protected]> net: mana: swap queue sets in mana_set_channels Build a replacement queue set before quiescing TX. After closing the TX/XDP gate and draining its readers, wait up to 120 seconds for ALL old TX queues before changing pointers, queue counts or RSS. > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > b/drivers/net/ethernet/microsoft/mana/mana_en.c > index 02cd5f7656ed8..10225c09f5b7f 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c [ ... ] > @@ -1094,6 +1132,58 @@ static void mana_free_queue_stats(struct > mana_port_context *apc) > apc->txq_stats = NULL; > } > > +/* Fold under RTNL after drain_stats writers quiesce. Clear drain_stats to > + * prevent double counting on rollback. > + */ > +static void mana_fold_rxq_stats(struct mana_port_context *apc, > + struct mana_rxq *rxq) > +{ > + struct mana_stats_rx *src = &rxq->drain_stats; > + struct mana_stats_rx *dst; > + unsigned int i; > + > + ASSERT_RTNL(); > + > + if (!apc->rxq_stats_ret || rxq->rxq_idx >= apc->max_queues) > + return; > + > + dst = &apc->rxq_stats_ret[rxq->rxq_idx]; > + > + u64_stats_update_begin(&dst->syncp); [Severity: High] Does this code expose a sequence counter race window to lockless readers? The mana_fold_rxq_stats() function runs in process context under RTNL, but it calls u64_stats_update_begin() without first disabling preemption or bottom-halves (softirqs). The u64_stats_sync API strictly requires writers to run with preemption disabled, and BH disabled if readers can be in softirq context. On 32-bit architectures, this macro increments a sequence counter. If a reader (like ndo_get_stats64 invoked via dev_get_stats() from a softirq or another thread) preempts the writer on the same CPU while the sequence counter is odd, the reader could spin infinitely in u64_stats_fetch_retry() waiting for it to become even, causing a hard deadlock. Should the update be wrapped in local_bh_disable() and local_bh_enable(), or use the _irqsave variant? > + dst->packets += src->packets; > + dst->bytes += src->bytes; > + dst->xdp_drop += src->xdp_drop; > + dst->xdp_tx += src->xdp_tx; > + dst->xdp_redirect += src->xdp_redirect; > + dst->pkt_len0_err += src->pkt_len0_err; > + for (i = 0; i < ARRAY_SIZE(dst->coalesced_cqe); i++) > + dst->coalesced_cqe[i] += src->coalesced_cqe[i]; > + u64_stats_update_end(&dst->syncp); > + > + src->packets = 0; > + src->bytes = 0; > + src->xdp_drop = 0; > + src->xdp_tx = 0; > + src->xdp_redirect = 0; > + src->pkt_len0_err = 0; > + for (i = 0; i < ARRAY_SIZE(src->coalesced_cqe); i++) > + src->coalesced_cqe[i] = 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
