From nobody Sun Feb 8 02:00:17 2026 Received: from mail-pg1-f202.google.com (mail-pg1-f202.google.com [209.85.215.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DED546FC3 for ; Fri, 25 Apr 2025 17:54:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745603686; cv=none; b=QV2o/fV4BDo6NZ8PWZdSCHcp1ldfbFqCsiD9XzTmyGE00Qgs4kSm7BkECLvZwf4Apy6O5mdi/hdSpwk0ibdAK9IL/tayaakIozyt9+2Ybzj192VPGK4zgc3FNYjhixgHGDUogjouWP13Pk1/RnQ2UCwXAGTWmgjFW0dhPtWVstY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745603686; c=relaxed/simple; bh=TDcDG4961RqBZwRUnI6OzSCYnyhIUCqoI2dBjdnGAnE=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=IlZWZwhnl2gRmku6cqp0QaJ0tTfZk5NXPVcaLFSzHX6iHiIzcY5wVo+XXYksvtMN0O5PJVxs1ajm3jyzinXNrJxDyrbkLOl/zGVyr1apD6sI3TBJI6F0MmX2Zlp/tbC/HQJXMuCdWtQT77NGKAzdlrEbNgUUDgeZD8OLosKLhtQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--brianvv.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=o9I2+0qP; arc=none smtp.client-ip=209.85.215.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--brianvv.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="o9I2+0qP" Received: by mail-pg1-f202.google.com with SMTP id 41be03b00d2f7-af534e796baso1427186a12.3 for ; Fri, 25 Apr 2025 10:54:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1745603682; x=1746208482; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=wUyDDDATMAbAlsjuyDcvLPYobzWIS7zLHCptp+fvkNY=; b=o9I2+0qPNcz8MGXrwOqP5+f3/L6j2tW8ByuLI5NZ5R1Mq667QY9ThTdKvOcovHQVeP 82wKHK7bM4go7iJguU1n07LfWLEhK/oYe/MOYouZt9jwWcdR5hcNq0Yt++HkcKKHQz6Y pzzjd3BZJRaZ0Sy43RcQJxE49mERYTYId+q9cuQ9Zc5YBjkOuw0oJiJ9f3o4IFvuFYOR 4ewDdmk1WHk/wk1trEPfM9OJdq2zGBtfwoDy07rXi977xRICmo2hK693cXqwCHoAu31z YCvqBpTrArIiWHYi8Wcz919cSv2/Rabq+IAuFN/ayJZlAHHrRedNTisV4hqXEi2szuuX 28YQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745603682; x=1746208482; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=wUyDDDATMAbAlsjuyDcvLPYobzWIS7zLHCptp+fvkNY=; b=GkCS55N7TCwXbqdigRmZzSQUv3J61rGH7kmu+WCQdTaXhGGOa5My/nuASXwfVEwux0 DIzxFRvZEJAO2RgxMX95WJpdS5DF6W8tSTnD8Op2bqszUFJl8bSH8UQgY9cBad/Ca/dQ 58uGodhGA6Vqfey88d5BKopOAweOKUBG3NrMlR+DDWbtYM3l+NOWuAw6u9oaTUXBD1M/ zzOfxqDykRfymQDqyqqQYnhyJBSQBMUBf6TP2Uz8l46ztha9eVgXpstlPrD32QgIYXsX GHhBWbF1X5zAS1EUeeFhADfGwrP+xckOJZBHUZD6/gekczBTAU4zZXuopzdNyZIpZ0iR 6j0Q== X-Forwarded-Encrypted: i=1; AJvYcCUff7IhgoU6gK7iTM3uv0s/Ftruz3kDENXijwihLgaH71NDuIgVm7pFqvvK3MsrJw+SxVRaLFUzvbGoCvI=@vger.kernel.org X-Gm-Message-State: AOJu0Yx84RMM5rv/Gtzmo9F6ycuiAIMNDgXRhJydHui4qxuwfTkoCPzK mEuNAg99HJ9KOv37n0KJf/OFUEyC88lM/CkriSDOxmFOLskuUOsdfOFeEsxqHc2wODNGYEL6P5j D7NLJfg== X-Google-Smtp-Source: AGHT+IEJMb0ZmvksR4COBHZqXiP7tE9JuIPcnR5oiaXvoL19IqL5V1QRPcjHRCuU+i31xkfihBjXX09nyPlC X-Received: from pfvf11.prod.google.com ([2002:a05:6a00:1acb:b0:739:485f:c33e]) (user=brianvv job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a21:1085:b0:1f5:5b2a:f641 with SMTP id adf61e73a8af0-2045b986d2amr4582036637.28.1745603681950; Fri, 25 Apr 2025 10:54:41 -0700 (PDT) Date: Fri, 25 Apr 2025 17:54:26 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Mailer: git-send-email 2.49.0.850.g28803427d3-goog Message-ID: <20250425175426.3353069-1-brianvv@google.com> Subject: [iwl-next PATCH] idpf: fix a race in txq wakeup From: Brian Vazquez To: Brian Vazquez , Tony Nguyen , Przemek Kitszel , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , intel-wired-lan@lists.osuosl.org Cc: David Decotigny , Anjali Singhai , Sridhar Samudrala , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, emil.s.tantilov@intel.com, Brian Vazquez , Josh Hay , Luigi Rizzo Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Add a helper function to correctly handle the lockless synchronization when the sender needs to block. The paradigm is if (no_resources()) { stop_queue(); barrier(); if (!no_resources()) restart_queue(); } netif_subqueue_maybe_stop already handles the paradigm correctly, but the code split the check for resources in three parts, the first one (descriptors) followed the protocol, but the other two (completions and tx_buf) were only doing the first part and so race prone. Luckly netif_subqueue_maybe_stop macro already allows you to use a function to evaluate the start/stop conditions so the fix only requires to pass the right helper function to evaluate all the conditions at once. The patch removes idpf_tx_maybe_stop_common since it's no longer needed and instead adjusts separetely the conditions for singleq and splitq. Note that idpf_rx_buf_hw_update doesn't need to check for resources since that will be covered in idpf_tx_splitq_frame. To reproduce: Reduce the threshold for pending completions to increase the chances of hitting this pause by locally changing the kernel: drivers/net/ethernet/intel/idpf/idpf_txrx.h -#define IDPF_TX_COMPLQ_OVERFLOW_THRESH(txcq) ((txcq)->desc_count >> 1) +#define IDPF_TX_COMPLQ_OVERFLOW_THRESH(txcq) ((txcq)->desc_count >> 4) Use pktgen to force the host to push small pkts very aggresively: ./pktgen_sample02_multiqueue.sh -i eth1 -s 100 -6 -d $IP -m $MAC \ -p 10000-10000 -t 16 -n 0 -v -x -c 64 Signed-off-by: Josh Hay Signed-off-by: Brian Vazquez Signed-off-by: Luigi Rizzo --- .../ethernet/intel/idpf/idpf_singleq_txrx.c | 9 ++-- drivers/net/ethernet/intel/idpf/idpf_txrx.c | 44 +++++++------------ drivers/net/ethernet/intel/idpf/idpf_txrx.h | 8 ---- 3 files changed, 21 insertions(+), 40 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c b/drivers/= net/ethernet/intel/idpf/idpf_singleq_txrx.c index c6b927fa9979..fb85270c69d6 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c @@ -364,15 +364,16 @@ netdev_tx_t idpf_tx_singleq_frame(struct sk_buff *skb, struct idpf_tx_buf *first; unsigned int count; __be16 protocol; - int csum, tso; + int csum, tso, needed; =20 count =3D idpf_tx_desc_count_required(tx_q, skb); if (unlikely(!count)) return idpf_tx_drop_skb(tx_q, skb); =20 - if (idpf_tx_maybe_stop_common(tx_q, - count + IDPF_TX_DESCS_PER_CACHE_LINE + - IDPF_TX_DESCS_FOR_CTX)) { + needed =3D count + IDPF_TX_DESCS_PER_CACHE_LINE + IDPF_TX_DESCS_FOR_CTX; + if (!netif_subqueue_maybe_stop(tx_q->netdev, tx_q->idx, + IDPF_DESC_UNUSED(tx_q), + needed, needed)) { idpf_tx_buf_hw_update(tx_q, tx_q->next_to_use, false); =20 u64_stats_update_begin(&tx_q->stats_sync); diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethe= rnet/intel/idpf/idpf_txrx.c index 970fa9e5c39b..cb41b6fcf03f 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c @@ -2184,6 +2184,19 @@ void idpf_tx_splitq_build_flow_desc(union idpf_tx_fl= ex_desc *desc, desc->flow.qw1.compl_tag =3D cpu_to_le16(params->compl_tag); } =20 +/* Global conditions to tell whether the txq (and related resources) + * has room to allow the use of "size" descriptors. + */ +static inline int txq_has_room(struct idpf_tx_queue *tx_q, u32 size) +{ + if (IDPF_DESC_UNUSED(tx_q) < size || + IDPF_TX_COMPLQ_PENDING(tx_q->txq_grp) > + IDPF_TX_COMPLQ_OVERFLOW_THRESH(tx_q->txq_grp->complq) || + IDPF_TX_BUF_RSV_LOW(tx_q)) + return 0; + return 1; +} + /** * idpf_tx_maybe_stop_splitq - 1st level check for Tx splitq stop conditio= ns * @tx_q: the queue to be checked @@ -2194,29 +2207,10 @@ void idpf_tx_splitq_build_flow_desc(union idpf_tx_f= lex_desc *desc, static int idpf_tx_maybe_stop_splitq(struct idpf_tx_queue *tx_q, unsigned int descs_needed) { - if (idpf_tx_maybe_stop_common(tx_q, descs_needed)) - goto out; - - /* If there are too many outstanding completions expected on the - * completion queue, stop the TX queue to give the device some time to - * catch up - */ - if (unlikely(IDPF_TX_COMPLQ_PENDING(tx_q->txq_grp) > - IDPF_TX_COMPLQ_OVERFLOW_THRESH(tx_q->txq_grp->complq))) - goto splitq_stop; - - /* Also check for available book keeping buffers; if we are low, stop - * the queue to wait for more completions - */ - if (unlikely(IDPF_TX_BUF_RSV_LOW(tx_q))) - goto splitq_stop; - - return 0; - -splitq_stop: - netif_stop_subqueue(tx_q->netdev, tx_q->idx); + if (netif_subqueue_maybe_stop(tx_q->netdev, tx_q->idx, + txq_has_room(tx_q, descs_needed), 1, 1)) + return 0; =20 -out: u64_stats_update_begin(&tx_q->stats_sync); u64_stats_inc(&tx_q->q_stats.q_busy); u64_stats_update_end(&tx_q->stats_sync); @@ -2242,12 +2236,6 @@ void idpf_tx_buf_hw_update(struct idpf_tx_queue *tx_= q, u32 val, nq =3D netdev_get_tx_queue(tx_q->netdev, tx_q->idx); tx_q->next_to_use =3D val; =20 - if (idpf_tx_maybe_stop_common(tx_q, IDPF_TX_DESC_NEEDED)) { - u64_stats_update_begin(&tx_q->stats_sync); - u64_stats_inc(&tx_q->q_stats.q_busy); - u64_stats_update_end(&tx_q->stats_sync); - } - /* Force memory writes to complete before letting h/w * know there are new descriptors to fetch. (Only * applicable for weak-ordered memory model archs, diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethe= rnet/intel/idpf/idpf_txrx.h index c779fe71df99..36a0f828a6f8 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h @@ -1049,12 +1049,4 @@ bool idpf_rx_singleq_buf_hw_alloc_all(struct idpf_rx= _queue *rxq, u16 cleaned_count); int idpf_tso(struct sk_buff *skb, struct idpf_tx_offload_params *off); =20 -static inline bool idpf_tx_maybe_stop_common(struct idpf_tx_queue *tx_q, - u32 needed) -{ - return !netif_subqueue_maybe_stop(tx_q->netdev, tx_q->idx, - IDPF_DESC_UNUSED(tx_q), - needed, needed); -} - #endif /* !_IDPF_TXRX_H_ */ --=20 2.49.0.850.g28803427d3-goog