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 smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 3EC72C3DA7F for ; Wed, 7 Aug 2024 17:05:56 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id CEAD280C77; Wed, 7 Aug 2024 17:05:55 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id 3huFuQsDbgcM; Wed, 7 Aug 2024 17:05:55 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.34; helo=ash.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 1DB3680D71 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1723050355; bh=KVNbsnpB5P7CPlk9mWYmVYf7DRex46w64FAD8tvofQA=; h=Date:From:To:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Cc:From; b=f+L5UkhyMepU0vi5JVwCAQrAwsALf/tNQoQfk4tMzvz6+NYdw6JspK6DR/ZU43VMh Owyr4+iorG3pdBcE7sKNf/SllOGBgpO3v2gToszRV1GJeJvjicazP6CovaAr6eI9fK a/QXr0ovM+8WZsNF9/dPgb4QWyt4KeHyTlpNGgyn/VSjuXmGPWutvcWvmys2L2x+H+ 9PGpPvsU0Bn2ZlpxDVuaFXXQbrQIOnYNKCfJ1xRiotRDpU5gdcvOgULm9C9yRBq3Mt MqF911cLx4nvDjuALMMB9k9LOxSBBf8wMQCQp6B754sv5Biy2qp2XjN43Ltepb507l wHVKWkvewYhOQ== Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp1.osuosl.org (Postfix) with ESMTP id 1DB3680D71; Wed, 7 Aug 2024 17:05:55 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id 022DC1BF30E for ; Wed, 7 Aug 2024 17:05:53 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id E0D6840621 for ; Wed, 7 Aug 2024 17:05:53 +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 qLM7TUe8FpKe for ; Wed, 7 Aug 2024 17:05:53 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=2604:1380:40e1:4800::1; helo=sin.source.kernel.org; envelope-from=horms@kernel.org; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp4.osuosl.org B0AC840611 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org B0AC840611 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by smtp4.osuosl.org (Postfix) with ESMTPS id B0AC840611 for ; Wed, 7 Aug 2024 17:05:52 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 2A5BECE0B90; Wed, 7 Aug 2024 17:05:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 094D4C4AF0D; Wed, 7 Aug 2024 17:05:44 +0000 (UTC) Date: Wed, 7 Aug 2024 18:05:42 +0100 From: Simon Horman To: Manoj Vishwanathan Message-ID: <20240807170542.GE3006561@kernel.org> References: <20240803182548.2932270-1-manojvishy@google.com> <20240805182159.3547482-1-manojvishy@google.com> <04affbd5-828a-4327-8b84-8767c1c139f1@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723050347; bh=aTViCyKmbUpyeYhUXvYqN+xUH5sZrDigFA7fo5bgqGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jCwa3mb/XrIlgV09o0SIJuC3Bxlnx0rEHSw1m4Oy2YR0d4feUg2BaRAuaKBYHEJ0b tsfJ6Ddn65nw0Myc9Hh0SPb1WuaovIr+Ei2w2gS2iTn4YvqnO2OufUS3QmZq6HLMFs qgRABE3wvB6kwfYsGFxxzwzIbY26OBNKKhgKH2qU8L+TcsQj2H7TD82ok/SmKQvTsF TimrbSSy3m3R+tGUEyj6jUbykPwYQs/uHHS3Q4QPwKrVQWomIBLr1vGREz6DWNvAMr Zl3BRtTEYe69XJnNW8/xCsThl2CiEhTwtUKMs18d6ams4mumPCy8veLi28vQoRclYR w3MtwR4EnMWPg== X-Mailman-Original-Authentication-Results: smtp4.osuosl.org; dmarc=pass (p=none dis=none) header.from=kernel.org X-Mailman-Original-Authentication-Results: smtp4.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=jCwa3mb/ Subject: Re: [Intel-wired-lan] [PATCH] [PATCH iwl-net] idpf: Acquire the lock before accessing the xn->salt X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Przemek Kitszel , linux-kernel@vger.kernel.org, Alexander Lobakin , Eric Dumazet , Tony Nguyen , netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, David Decotigny , "David S. Miller" Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" On Wed, Aug 07, 2024 at 06:58:59AM -0700, Manoj Vishwanathan wrote: > Thanks Przemek & Olek for your quick feedback and responses. > Hi Olek, > I can add more details about the issue we faced in the commit message. > The bug we had here was a virtchnl delay leading to the xn->salt > mismatch. This could be due to several factors including default CPU > bounded kworker workqueue for virtchnl message processing being > starved by aggressive userspace load causing the virtchnl to be > delayed. While debugging this issue, this locking order appeared like > a potential issue, hence the change was made. > But, this change is more a clean up we felt based on concurrent access > to the virtchnl transaction struct and does not fix the issue. This is > more of the patch to do the right thing before we access the "xn". > I wanted to start with a first patch to the community for acceptance > followed by a series of other patches that are general clean up or > improvements to IDPF in general. Will follow with with [PATCH v3] Still, I am a little confused about the protection offered to xn->salt. My analysis is as follows, where guarded is used loosely to mean the lock is held. * In idpf_vc_xn_pop_free() it is guarded by vcxn_mngr->xn_bm_lock. * In idpf_vc_xn_exec() it is guarded by: 1. vcxn_mngr->xn_bm_lock when idpf_vc_xn_pop_free is called 2. idpf_vc_xn_lock, otherwise * And with this patch, in idpf_vc_xn_forward_reply it is guarded by idpf_vc_xn_lock(). This doesn't seem entirely consistent. Also, please don't top-post on Kernel mailing lists. ... 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 A23FD12F398; Wed, 7 Aug 2024 17:05:47 +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=1723050347; cv=none; b=lNZCdVA7TWVupWj7wE6c9aRcXrIJl2TXMI8T1vTQHsCiE+BZ/2xLQ3c8uXCEI0NvqjirRCtuvvEWR8/2GaBMFDNwhxQELEPyM4aauqnqjp4oysNdXK/RW34Ro7wsts1yf5ma+xGZeWjMuKLL1xDPzdaJ8KH1YhSeIoii3pkeAcA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723050347; c=relaxed/simple; bh=aTViCyKmbUpyeYhUXvYqN+xUH5sZrDigFA7fo5bgqGQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ilwbn7rwrVLz5pHARvpo1Z/igCQZGGLMk8Nb6hVbAM2e07cwiV79fVH5ebN454Q1IQdftswHXAkGDkPD0fEkO6bwghi8EsMxzjAS5waEHYVX/nJFZn3ojsFwz7GDFJ7GxwUtzktj0Mx0G1oXwD+KUdIr2PGKjTCHdkoTiH/rOH8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=jCwa3mb/; 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="jCwa3mb/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 094D4C4AF0D; Wed, 7 Aug 2024 17:05:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723050347; bh=aTViCyKmbUpyeYhUXvYqN+xUH5sZrDigFA7fo5bgqGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jCwa3mb/XrIlgV09o0SIJuC3Bxlnx0rEHSw1m4Oy2YR0d4feUg2BaRAuaKBYHEJ0b tsfJ6Ddn65nw0Myc9Hh0SPb1WuaovIr+Ei2w2gS2iTn4YvqnO2OufUS3QmZq6HLMFs qgRABE3wvB6kwfYsGFxxzwzIbY26OBNKKhgKH2qU8L+TcsQj2H7TD82ok/SmKQvTsF TimrbSSy3m3R+tGUEyj6jUbykPwYQs/uHHS3Q4QPwKrVQWomIBLr1vGREz6DWNvAMr Zl3BRtTEYe69XJnNW8/xCsThl2CiEhTwtUKMs18d6ams4mumPCy8veLi28vQoRclYR w3MtwR4EnMWPg== Date: Wed, 7 Aug 2024 18:05:42 +0100 From: Simon Horman To: Manoj Vishwanathan Cc: Alexander Lobakin , netdev@vger.kernel.org, David Decotigny , linux-kernel@vger.kernel.org, Tony Nguyen , Przemek Kitszel , "David S. Miller" , Eric Dumazet , intel-wired-lan@lists.osuosl.org Subject: Re: [Intel-wired-lan] [PATCH] [PATCH iwl-net] idpf: Acquire the lock before accessing the xn->salt Message-ID: <20240807170542.GE3006561@kernel.org> References: <20240803182548.2932270-1-manojvishy@google.com> <20240805182159.3547482-1-manojvishy@google.com> <04affbd5-828a-4327-8b84-8767c1c139f1@intel.com> Precedence: bulk X-Mailing-List: linux-kernel@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: On Wed, Aug 07, 2024 at 06:58:59AM -0700, Manoj Vishwanathan wrote: > Thanks Przemek & Olek for your quick feedback and responses. > Hi Olek, > I can add more details about the issue we faced in the commit message. > The bug we had here was a virtchnl delay leading to the xn->salt > mismatch. This could be due to several factors including default CPU > bounded kworker workqueue for virtchnl message processing being > starved by aggressive userspace load causing the virtchnl to be > delayed. While debugging this issue, this locking order appeared like > a potential issue, hence the change was made. > But, this change is more a clean up we felt based on concurrent access > to the virtchnl transaction struct and does not fix the issue. This is > more of the patch to do the right thing before we access the "xn". > I wanted to start with a first patch to the community for acceptance > followed by a series of other patches that are general clean up or > improvements to IDPF in general. Will follow with with [PATCH v3] Still, I am a little confused about the protection offered to xn->salt. My analysis is as follows, where guarded is used loosely to mean the lock is held. * In idpf_vc_xn_pop_free() it is guarded by vcxn_mngr->xn_bm_lock. * In idpf_vc_xn_exec() it is guarded by: 1. vcxn_mngr->xn_bm_lock when idpf_vc_xn_pop_free is called 2. idpf_vc_xn_lock, otherwise * And with this patch, in idpf_vc_xn_forward_reply it is guarded by idpf_vc_xn_lock(). This doesn't seem entirely consistent. Also, please don't top-post on Kernel mailing lists. ...