From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AE66CC3ABAC for ; Fri, 2 May 2025 09:39:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 520234154E; Fri, 2 May 2025 09:39:57 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 121dpQZ5shTQ; Fri, 2 May 2025 09:39:56 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.142; helo=lists1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org C00E941602 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1746178796; bh=//GMkQzxZC0KMpU8i+Shvim8+jbl8o7IVgZ1cwJWIwc=; h=Date:From:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=c4vxlfyDW3BvbDuekzubgvdtDbTtKQ9xnkkDmChKZWz8SWmy3MRXVgLahGPdinHYk mYp5SPHPTEjRHQMfsJIyatrdJp31F7/OgbYXq+DOaQT9HsIQuRdK9Xbb6a1jFrLWLT lYv2qCAQDjLfjorNtRl/QQmeiBnI4kORPMGMxnJ6oq0hG2aqE7ArxwoOk5aFbyFR/b wDdIbgMvPjjc6mp8P29mJYlpHOFwGv66kAijZvwGnMeIA7QPWD0dTotQo0u6/MKr3B 3eFgztV99ffp1ScsCIp0DueJlbuOYGDC4lg0lfEspwMO0kw088D60f8rVw8UeyG9Dh NryASKx08UEKA== Received: from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142]) by smtp4.osuosl.org (Postfix) with ESMTP id C00E941602; Fri, 2 May 2025 09:39:56 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists1.osuosl.org (Postfix) with ESMTP id 6FFCE229 for ; Fri, 2 May 2025 09:39:55 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 554F060B9F for ; Fri, 2 May 2025 09:39:55 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id v4U2VEtvz8NG for ; Fri, 2 May 2025 09:39:50 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=172.105.4.254; helo=tor.source.kernel.org; envelope-from=horms@kernel.org; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp3.osuosl.org 55518607A8 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp3.osuosl.org 55518607A8 Received: from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254]) by smtp3.osuosl.org (Postfix) with ESMTPS id 55518607A8 for ; Fri, 2 May 2025 09:39:50 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 3FA9160007; Fri, 2 May 2025 09:39:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 27526C4CEE4; Fri, 2 May 2025 09:39:43 +0000 (UTC) Date: Fri, 2 May 2025 10:39:41 +0100 From: Simon Horman To: Brian Vazquez Cc: Brian Vazquez , Tony Nguyen , Przemek Kitszel , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , intel-wired-lan@lists.osuosl.org, David Decotigny , Anjali Singhai , Sridhar Samudrala , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, emil.s.tantilov@intel.com, Jacob Keller , Madhu Chittim , Josh Hay , Luigi Rizzo Message-ID: <20250502093941.GG3339421@horms.kernel.org> References: <20250501170617.1121247-1-brianvv@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250501170617.1121247-1-brianvv@google.com> X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1746178788; bh=OTcU+TyYte9xshVPz57Q6Apwcs2oIlxw0u2RikVAXJ8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=F/qS5AeO3aUkgzYbtK422mwKQ8LGFgid2UYoNMG4Ql9L9yVYEedacS7T5dVHaFHSj R1IRASQL0gusPIM5WdsMxwG47IsOj5gNacezG/2G0pdDDvyIrf+gmqpQM8BxUnia7x xUlRARHT8okKYTivPae0gR394TpXZT9ucnvgSBpDO8ocV2c2JGOKjth4BHlab7EQ3o 84P5LZ5Zoayx0KmXUeXTaJop5UERRT5HUNkhsdnKEB13JsJ9utMyUJUhXQRTUPyVVe Fkqy5iM6n+fdeF9jQY/1uASQ8W0dPsT0nfMjTdSXB54PzBbiYNJU3jjx9Txn2hWxpx wCZUbdLa/DiLw== X-Mailman-Original-Authentication-Results: smtp3.osuosl.org; dmarc=pass (p=quarantine dis=none) header.from=kernel.org X-Mailman-Original-Authentication-Results: smtp3.osuosl.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=F/qS5AeO Subject: Re: [Intel-wired-lan] [iwl-net PATCH v3] idpf: fix a race in txq wakeup X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" On Thu, May 01, 2025 at 05:06:17PM +0000, Brian Vazquez wrote: > 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. > > Luckily netif_subqueue_maybe_stop macro already allows you to use a > function to evaluate the start/stop conditions so the fix only requires > 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 separately the conditions for singleq and splitq. > > Note that idpf_tx_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 changing your 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 aggressively: > > ./pktgen_sample02_multiqueue.sh -i eth1 -s 100 -6 -d $IP -m $MAC \ > -p 10000-10000 -t 16 -n 0 -v -x -c 64 > > Fixes: 6818c4d5b3c2 ("idpf: add splitq start_xmit") > Reviewed-by: Jacob Keller > Reviewed-by: Madhu Chittim > Signed-off-by: Josh Hay > Signed-off-by: Brian Vazquez > Signed-off-by: Luigi Rizzo > --- > v3: > - Fix typo in commit message > v2: > - Fix typos > - Fix RCT in singleq function > - No inline in c files > - Submit to iwl-net and add Fixes tag Thanks Brian, This version looks good to me. Reviewed-by: Simon Horman From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EFD631D554; Fri, 2 May 2025 09:39:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746178789; cv=none; b=ob+jM2cL5CgCPArWJR3hSUwdVvYlWCMGzFu1H+E2lpVk29+34IMV2hlcILuxwh7FvxTGOU38Pu5sLDS0lcZVtD70nmBrS7iPKgrnJopD+WaD1vlSAptB929Ybuk++D7D2vtJIMTruWEHeSCM2QacVJ/tLP96kZjxE5pCF3JiS4g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746178789; c=relaxed/simple; bh=OTcU+TyYte9xshVPz57Q6Apwcs2oIlxw0u2RikVAXJ8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cZlzYRLEpomjrWSEC/Pv7xSTJllPiFCnD0q5/QIK5YOdJ67HNfKL6Lnlw1CG3flnG0cZO89kGIsGQbHMsZc/B3pU/RpAZgjgIS0qi/AvzzbFUl97Wvz6arWHjRcSsUz5NAqQ0M1HZY8aXLRPQ74ICCgGCx29EvdybPrn1QKswqM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=F/qS5AeO; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="F/qS5AeO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 27526C4CEE4; Fri, 2 May 2025 09:39:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1746178788; bh=OTcU+TyYte9xshVPz57Q6Apwcs2oIlxw0u2RikVAXJ8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=F/qS5AeO3aUkgzYbtK422mwKQ8LGFgid2UYoNMG4Ql9L9yVYEedacS7T5dVHaFHSj R1IRASQL0gusPIM5WdsMxwG47IsOj5gNacezG/2G0pdDDvyIrf+gmqpQM8BxUnia7x xUlRARHT8okKYTivPae0gR394TpXZT9ucnvgSBpDO8ocV2c2JGOKjth4BHlab7EQ3o 84P5LZ5Zoayx0KmXUeXTaJop5UERRT5HUNkhsdnKEB13JsJ9utMyUJUhXQRTUPyVVe Fkqy5iM6n+fdeF9jQY/1uASQ8W0dPsT0nfMjTdSXB54PzBbiYNJU3jjx9Txn2hWxpx wCZUbdLa/DiLw== Date: Fri, 2 May 2025 10:39:41 +0100 From: Simon Horman To: Brian Vazquez Cc: Brian Vazquez , Tony Nguyen , Przemek Kitszel , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , intel-wired-lan@lists.osuosl.org, David Decotigny , Anjali Singhai , Sridhar Samudrala , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, emil.s.tantilov@intel.com, Jacob Keller , Madhu Chittim , Josh Hay , Luigi Rizzo Subject: Re: [iwl-net PATCH v3] idpf: fix a race in txq wakeup Message-ID: <20250502093941.GG3339421@horms.kernel.org> References: <20250501170617.1121247-1-brianvv@google.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250501170617.1121247-1-brianvv@google.com> On Thu, May 01, 2025 at 05:06:17PM +0000, Brian Vazquez wrote: > 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. > > Luckily netif_subqueue_maybe_stop macro already allows you to use a > function to evaluate the start/stop conditions so the fix only requires > 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 separately the conditions for singleq and splitq. > > Note that idpf_tx_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 changing your 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 aggressively: > > ./pktgen_sample02_multiqueue.sh -i eth1 -s 100 -6 -d $IP -m $MAC \ > -p 10000-10000 -t 16 -n 0 -v -x -c 64 > > Fixes: 6818c4d5b3c2 ("idpf: add splitq start_xmit") > Reviewed-by: Jacob Keller > Reviewed-by: Madhu Chittim > Signed-off-by: Josh Hay > Signed-off-by: Brian Vazquez > Signed-off-by: Luigi Rizzo > --- > v3: > - Fix typo in commit message > v2: > - Fix typos > - Fix RCT in singleq function > - No inline in c files > - Submit to iwl-net and add Fixes tag Thanks Brian, This version looks good to me. Reviewed-by: Simon Horman