From patchwork Tue Feb 4 17:52:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 13959536 X-Patchwork-Delegate: kuba@kernel.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) (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 7B6D421638B for ; Tue, 4 Feb 2025 17:52:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691576; cv=none; b=THGbLekABDr52s5BPp8A/lw9EWxpUBGBiJZhhyJhGdPnIXE9cpYZzBrey8+y56lJvzpAGltO+df8qpzojgShcSZK+kn8rN9OcI2H6SdKdKjeOvCMJj7vGPpuHXhUFZbhhlKBwK8WY+BVdB4/984QOSr/zUE2XQhAYu92ZIUXmzo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691576; c=relaxed/simple; bh=DjRRCNPcLPWjuJq1XFZ/jGqru4FOEvkn8qDPdjG7ZKE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=kistiFfZ7XG9XAQ+Kt5mbjcphDUBqSW7En+lrTf2gKVajxD6CwPVOkT+YEzgO4N2AtggiGKRDPyrJtM8dDZjPwFXq6lPNO7RnlSHukBPLy/xY0BOcRq82/muzH+sCeT7ChR5SBFQ0kqay/+1UrBSQiYYG0QS6YRw5HxxwOaFSGw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=e3h9Tiez; arc=none smtp.client-ip=198.175.65.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="e3h9Tiez" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1738691575; x=1770227575; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=DjRRCNPcLPWjuJq1XFZ/jGqru4FOEvkn8qDPdjG7ZKE=; b=e3h9TiezhLCXAS6Bl3K1stMFQjUHilOnd4vu1T2Vc0MVFRd1wYoOPKVU fUq3A+jzHmZGw3Xxhtg9+UAe8uJlmEghgNLIXemfBLAwW+UBw3uD9KwS+ gszXKO57Ww4Y0Hwjzsvv/LxmaqbPkcACsFyrP0h0Y9cMe9vAnTcg5RSVY 9lJ/LOJ5e1CLDVv7jGhddFBdorbxyL7acDYg3Nb9oDFniRsRyy+/gkADm vRX+mVWBxmxDrqAGqytYgQeo+CSxkuI+7x9n+hAh7EleAN9SqrAxEFZzz ga+53NeT7zLaYfBvZvtrc0OVXdHCezCpK2/CKl+b/pJ+I6o1erkbAfMRa g==; X-CSE-ConnectionGUID: Bb9ZqvNMQN6dXi+pnvB+mg== X-CSE-MsgGUID: OAiR3utOQc+HcDkhtm4HOA== X-IronPort-AV: E=McAfee;i="6700,10204,11336"; a="39371875" X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="39371875" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2025 09:52:54 -0800 X-CSE-ConnectionGUID: 7mdMjiabSVS+VCPErw1DRw== X-CSE-MsgGUID: +MHDgjemSuqPDMEu8JTX+w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="110652387" Received: from anguy11-upstream.jf.intel.com ([10.166.9.133]) by orviesa006.jf.intel.com with ESMTP; 04 Feb 2025 09:52:53 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, edumazet@google.com, andrew+netdev@lunn.ch, netdev@vger.kernel.org Cc: Wander Lairson Costa , anthony.l.nguyen@intel.com, rostedt@goodmis.org, clrkwllms@kernel.org, bigeasy@linutronix.de, jgarzik@redhat.com, yuma@redhat.com, linux-rt-devel@lists.linux.dev, Rafal Romanowski Subject: [PATCH net 1/4] igb: narrow scope of vfs_lock in SR-IOV cleanup Date: Tue, 4 Feb 2025 09:52:37 -0800 Message-ID: <20250204175243.810189-2-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20250204175243.810189-1-anthony.l.nguyen@intel.com> References: <20250204175243.810189-1-anthony.l.nguyen@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org From: Wander Lairson Costa The adapter->vfs_lock currently protects critical sections shared between igb_disable_sriov() and igb_msg_task(). Since igb_msg_task() - which is invoked solely by the igb_msix_other() ISR-only proceeds when adapter->vfs_allocated_count > 0, we can reduce the lock scope further. By moving the assignment adapter->vfs_allocated_count = 0 to the start of the cleanup code in igb_disable_sriov(), we can restrict the spinlock protection solely to this assignment. This change removes kfree() calls from within the locked section, simplifying lock management. Once kfree() is outside the vfs_lock scope, it becomes possible to safely convert vfs_lock to a raw_spin_lock. Signed-off-by: Wander Lairson Costa Tested-by: Rafal Romanowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/igb/igb_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index d368b753a467..77571f6fdbfd 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -3708,12 +3708,12 @@ static int igb_disable_sriov(struct pci_dev *pdev, bool reinit) msleep(500); } spin_lock_irqsave(&adapter->vfs_lock, flags); + adapter->vfs_allocated_count = 0; + spin_unlock_irqrestore(&adapter->vfs_lock, flags); kfree(adapter->vf_mac_list); adapter->vf_mac_list = NULL; kfree(adapter->vf_data); adapter->vf_data = NULL; - adapter->vfs_allocated_count = 0; - spin_unlock_irqrestore(&adapter->vfs_lock, flags); wr32(E1000_IOVCTL, E1000_IOVCTL_REUSE_VFQ); wrfl(); msleep(100); From patchwork Tue Feb 4 17:52:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 13959540 X-Patchwork-Delegate: kuba@kernel.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) (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 548F22165E4 for ; Tue, 4 Feb 2025 17:52:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691579; cv=none; b=N2ouyhO4fQ4WDVADLmJ/IloRCMScsr3KbGZOOVd0hlI9vvFrvbAJuUGqVdj8LNuMDiInW5d0Gl9AACvU81kQdIYBvFapx5N0nqKxQ6PxYeUuDw7gsN9dpYlOLuW1wkSKczjPuuKUowJaFz3xkKc4gkHWGrf5cV8gYr9TsbfalHY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691579; c=relaxed/simple; bh=Ij5+AAvOEp0oI4NlaPmZaEL3V4o/dX6CD6vtMOaL+9M=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bgQ0XciEPL1opko2By/FLuKjTPO+Nla0yvQPeYkz0KiHZ7PmPhKZICFTsPGu+DZ6drnlD+KtftqfEtjPN2uzKRGsPUXrzv/RoaZR1LqYO7SZ2vetxmhbWIioM0s2oyRKmuWB2XFaW/5KusdQ3SxyL7bQ8urcOComw7IJTQJywac= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=G+H5aNWt; arc=none smtp.client-ip=198.175.65.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="G+H5aNWt" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1738691577; x=1770227577; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=Ij5+AAvOEp0oI4NlaPmZaEL3V4o/dX6CD6vtMOaL+9M=; b=G+H5aNWtcgsHXbWrOJeZvpwplWkkTuVIxMwk/JSRklXbd0EuMeFj4PXP LBPKX0Yz8Y6Pm6+4hoUe5VKSehG4elyKOFVfSxRH26P1ZCDE01N854IHv drSgCWIlm4GW3bEDF106SjJknDJOtnBzAqspmyPlZWnEVKTxElLNGWMEj WlhGS8+pIpg+H3v6d6BlmJM8f0hpdvV4EQoTkOnQEhvSNvJT4kCRv7kUJ 4WPqQ9mAs7sQ7pEdA7u3igFSQ/GUhHbBh9xPcOcnRCeGZRPFv6mXu9NYL s8s5ELGVoDrVs3TduLUTc9ceSOx6yoAdoF/QT+Q3O7KoWsVtY+j2Qj/8b A==; X-CSE-ConnectionGUID: X1fFGUzkT/C8MapDN73oZA== X-CSE-MsgGUID: WedJB6wHS1i6FT8kzPdPeQ== X-IronPort-AV: E=McAfee;i="6700,10204,11336"; a="39371886" X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="39371886" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2025 09:52:54 -0800 X-CSE-ConnectionGUID: wKHizxhkSN24MAHp/mtT9g== X-CSE-MsgGUID: rypIaQFbQb67jBu8T0tSfA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="110652390" Received: from anguy11-upstream.jf.intel.com ([10.166.9.133]) by orviesa006.jf.intel.com with ESMTP; 04 Feb 2025 09:52:53 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, edumazet@google.com, andrew+netdev@lunn.ch, netdev@vger.kernel.org Cc: Wander Lairson Costa , anthony.l.nguyen@intel.com, rostedt@goodmis.org, clrkwllms@kernel.org, bigeasy@linutronix.de, jgarzik@redhat.com, yuma@redhat.com, linux-rt-devel@lists.linux.dev, Clark Williams , Rafal Romanowski Subject: [PATCH net 2/4] igb: introduce raw vfs_lock to igb_adapter Date: Tue, 4 Feb 2025 09:52:38 -0800 Message-ID: <20250204175243.810189-3-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20250204175243.810189-1-anthony.l.nguyen@intel.com> References: <20250204175243.810189-1-anthony.l.nguyen@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org From: Wander Lairson Costa This change adds a raw_spinlock for the vfs_lock to the igb_adapter structure, enabling its use in both interrupt and preemptible contexts. This is essential for upcoming modifications to split igb_msg_task() into interrupt-safe and preemptible-safe parts. The motivation for this change stems from the need to modify igb_msix_other() to run in interrupt context under PREEMPT_RT. Currently, igb_msg_task() contains a code path that invokes kcalloc() with the GFP_ATOMIC flag. However, on PREEMPT_RT, GFP_ATOMIC is not honored, making it unsafe to call allocation functions in interrupt context. By introducing this raw spinlock, we can safely acquire the lock in both contexts, paving the way for the necessary restructuring of igb_msg_task(). Signed-off-by: Wander Lairson Costa Suggested-by: Clark Williams Tested-by: Rafal Romanowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/igb/igb.h | 4 ++ drivers/net/ethernet/intel/igb/igb_main.c | 51 ++++++++++++++++++++--- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 02f340280d20..30a188c5710e 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -673,6 +673,10 @@ struct igb_adapter { struct vf_mac_filter *vf_mac_list; /* lock for VF resources */ spinlock_t vfs_lock; +#ifdef CONFIG_PREEMPT_RT + /* Used to lock VFS in interrupt context under PREEMPT_RT */ + raw_spinlock_t raw_vfs_lock; +#endif }; /* flags controlling PTP/1588 function */ diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 77571f6fdbfd..4e75c88f6214 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -3657,6 +3657,47 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent) return err; } +#ifdef CONFIG_PREEMPT_RT +static __always_inline void vfs_lock_init(struct igb_adapter *adapter) +{ + spin_lock_init(&adapter->vfs_lock); + raw_spin_lock_init(&adapter->raw_vfs_lock); +} + +static __always_inline void vfs_lock_irqsave(struct igb_adapter *adapter, + unsigned long *flags) +{ + /* + * Remember that under PREEMPT_RT spin_lock_irqsave + * ignores the flags parameter + */ + spin_lock_irqsave(&adapter->vfs_lock, *flags); + raw_spin_lock_irqsave(&adapter->raw_vfs_lock, *flags); +} + +static __always_inline void vfs_unlock_irqrestore(struct igb_adapter *adapter, + unsigned long flags) +{ + raw_spin_unlock_irqrestore(&adapter->raw_vfs_lock, flags); + spin_unlock_irqrestore(&adapter->vfs_lock, flags); +} +#else +static __always_inline void vfs_lock_init(struct igb_adapter *adapter) +{ + spin_lock_init(&adapter->vfs_lock); +} + +static __always_inline void vfs_lock_irqsave(struct igb_adapter *adapter, unsigned long *flags) +{ + spin_lock_irqsave(&adapter->vfs_lock, *flags); +} + +static __always_inline void vfs_unlock_irqrestore(struct igb_adapter *adapter, unsigned long flags) +{ + spin_unlock_irqrestore(&adapter->vfs_lock, flags); +} +#endif + #ifdef CONFIG_PCI_IOV static int igb_sriov_reinit(struct pci_dev *dev) { @@ -3707,9 +3748,9 @@ static int igb_disable_sriov(struct pci_dev *pdev, bool reinit) pci_disable_sriov(pdev); msleep(500); } - spin_lock_irqsave(&adapter->vfs_lock, flags); + vfs_lock_irqsave(adapter, &flags); adapter->vfs_allocated_count = 0; - spin_unlock_irqrestore(&adapter->vfs_lock, flags); + vfs_unlock_irqrestore(adapter, flags); kfree(adapter->vf_mac_list); adapter->vf_mac_list = NULL; kfree(adapter->vf_data); @@ -4042,7 +4083,7 @@ static int igb_sw_init(struct igb_adapter *adapter) spin_lock_init(&adapter->stats64_lock); /* init spinlock to avoid concurrency of VF resources */ - spin_lock_init(&adapter->vfs_lock); + vfs_lock_init(adapter); #ifdef CONFIG_PCI_IOV switch (hw->mac.type) { case e1000_82576: @@ -8078,7 +8119,7 @@ static void igb_msg_task(struct igb_adapter *adapter) unsigned long flags; u32 vf; - spin_lock_irqsave(&adapter->vfs_lock, flags); + vfs_lock_irqsave(adapter, &flags); for (vf = 0; vf < adapter->vfs_allocated_count; vf++) { /* process any reset requests */ if (!igb_check_for_rst(hw, vf)) @@ -8092,7 +8133,7 @@ static void igb_msg_task(struct igb_adapter *adapter) if (!igb_check_for_ack(hw, vf)) igb_rcv_ack_from_vf(adapter, vf); } - spin_unlock_irqrestore(&adapter->vfs_lock, flags); + vfs_unlock_irqrestore(adapter, flags); } /** From patchwork Tue Feb 4 17:52:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 13959538 X-Patchwork-Delegate: kuba@kernel.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) (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 548542165E0 for ; Tue, 4 Feb 2025 17:52:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691578; cv=none; b=eekFE5Igt7e4BjIKuT+2BJyZAn5NGnG6Z5wpBCv7QksGZgWMI9l5slrcFRdvR6zaI44qahALC6RC3zPueiE1a3of0uLcwJA0YBaBFwOxD18xqk75ksX4jPICwvusSySal9V9D8PG6P4KmM9KEhruQtGjixM60YJ0YxIGbpJ7Nh8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691578; c=relaxed/simple; bh=FYdWzTpcxPkf0D2vrnuywRxJdpODKh0DW9jB4ZCwYoY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=tnPRST8Toh6S2AdoGd8T2AqLNQH7WbBKvb8FFqv/4PV8KduST3i5yZZ/MdYV1IsJbdiiOxGW6yPz9JB9x04V7tRY1nXb7aPwk0s+Fxb+OzO70iaEKzVoiWJ+fb0ukh9xXs3GKWMDrUHrGbL6vZPDQRd06AWkwajfdNFHB6tbvf0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=GQJ1HVjM; arc=none smtp.client-ip=198.175.65.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="GQJ1HVjM" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1738691577; x=1770227577; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=FYdWzTpcxPkf0D2vrnuywRxJdpODKh0DW9jB4ZCwYoY=; b=GQJ1HVjM5IKxgVrSFCrqg1SSqnsZB0zSlSQu/b4GlN2u1dJzQG4xTdDm 696LJVOSu19qtH7sb12G04qhr8jx28hnRSnVoQKk0yWRcZ9S5yqi2UyBV MA8JVmu+InHjhzdMVGyDU9JlHWCEL2dzlcutHT/RlS2p6luc6CI7Ynk4C mUD4obFSkNMrfDy8TBOX6FDSe2hpjnu6BF12HwpGUtRWUjgYnV6Vw7XCB CtBWfRfUfiJXz+SqGrNUjh720nHV9mzy+298sfKaC0x5inHAzZUeykzqv DJCNJQIzvIe6SD+rNgHSPBD8YJLpavvCkhBGtHxdJK+aReYrDuo7M6O4c Q==; X-CSE-ConnectionGUID: P54LWji2QauNo8bfcrFodQ== X-CSE-MsgGUID: g2s8Nc5cRdekB7qqztlHBQ== X-IronPort-AV: E=McAfee;i="6700,10204,11336"; a="39371896" X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="39371896" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2025 09:52:54 -0800 X-CSE-ConnectionGUID: 3W+/JMlsRw+s2Dj99EDLPQ== X-CSE-MsgGUID: 2IRSlQ1hSWGEeO+Ja6xtuw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="110652396" Received: from anguy11-upstream.jf.intel.com ([10.166.9.133]) by orviesa006.jf.intel.com with ESMTP; 04 Feb 2025 09:52:53 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, edumazet@google.com, andrew+netdev@lunn.ch, netdev@vger.kernel.org Cc: Wander Lairson Costa , anthony.l.nguyen@intel.com, rostedt@goodmis.org, clrkwllms@kernel.org, bigeasy@linutronix.de, jgarzik@redhat.com, yuma@redhat.com, linux-rt-devel@lists.linux.dev, Rafal Romanowski Subject: [PATCH net 3/4] igb: split igb_msg_task() Date: Tue, 4 Feb 2025 09:52:39 -0800 Message-ID: <20250204175243.810189-4-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20250204175243.810189-1-anthony.l.nguyen@intel.com> References: <20250204175243.810189-1-anthony.l.nguyen@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org From: Wander Lairson Costa From the perspective of PREEMPT_RT, igb_msg_task() invokes functions that are a mix of IRQ-safe and non-IRQ-safe operations. To address this, we separate igb_msg_task() into distinct IRQ-safe and preemptible-safe components. This is a preparatory step for upcoming commits, where the igb_msix_other interrupt handler will be split into IRQ and threaded handlers, each invoking the appropriate part of the newly divided igb_msg_task(). Signed-off-by: Wander Lairson Costa Tested-by: Rafal Romanowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/igb/igb_main.c | 88 +++++++++++++++++++++-- 1 file changed, 81 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 4e75c88f6214..6d590192c27f 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -146,6 +146,8 @@ static int igb_vlan_rx_kill_vid(struct net_device *, __be16, u16); static void igb_restore_vlan(struct igb_adapter *); static void igb_rar_set_index(struct igb_adapter *, u32); static void igb_ping_all_vfs(struct igb_adapter *); +static void igb_msg_task_irq_safe(struct igb_adapter *adapter); +static void igb_msg_task_preemptible_safe(struct igb_adapter *adapter); static void igb_msg_task(struct igb_adapter *); static void igb_vmm_control(struct igb_adapter *); static int igb_set_vf_mac(struct igb_adapter *, int, unsigned char *); @@ -3681,6 +3683,30 @@ static __always_inline void vfs_unlock_irqrestore(struct igb_adapter *adapter, raw_spin_unlock_irqrestore(&adapter->raw_vfs_lock, flags); spin_unlock_irqrestore(&adapter->vfs_lock, flags); } + +static __always_inline void vfs_spin_lock_irqsave(struct igb_adapter *adapter, + unsigned long *flags) +{ + spin_lock_irqsave(&adapter->vfs_lock, *flags); +} + +static __always_inline void vfs_spin_unlock_irqrestore(struct igb_adapter *adapter, + unsigned long flags) +{ + spin_unlock_irqrestore(&adapter->vfs_lock, flags); +} + +static __always_inline void vfs_raw_spin_lock_irqsave(struct igb_adapter *adapter, + unsigned long *flags) +{ + raw_spin_lock_irqsave(&adapter->raw_vfs_lock, *flags); +} + +static __always_inline void vfs_raw_spin_unlock_irqrestore(struct igb_adapter *adapter, + unsigned long flags) +{ + raw_spin_unlock_irqrestore(&adapter->raw_vfs_lock, flags); +} #else static __always_inline void vfs_lock_init(struct igb_adapter *adapter) { @@ -3696,6 +3722,30 @@ static __always_inline void vfs_unlock_irqrestore(struct igb_adapter *adapter, u { spin_unlock_irqrestore(&adapter->vfs_lock, flags); } + +static __always_inline void vfs_spin_lock_irqsave(struct igb_adapter *adapter, + unsigned long *flags) +{ + spin_lock_irqsave(&adapter->vfs_lock, *flags); +} + +static __always_inline void vfs_spin_unlock_irqrestore(struct igb_adapter *adapter, + unsigned long flags) +{ + spin_unlock_irqrestore(&adapter->vfs_lock, flags); +} + +static __always_inline void vfs_raw_spin_lock_irqsave(struct igb_adapter *adapter, + unsigned long *flags) +{ + spin_lock_irqsave(&adapter->vfs_lock, *flags); +} + +static __always_inline void vfs_raw_spin_unlock_irqrestore(struct igb_adapter *adapter, + unsigned long flags) +{ + spin_unlock_irqrestore(&adapter->vfs_lock, flags); +} #endif #ifdef CONFIG_PCI_IOV @@ -8113,27 +8163,51 @@ static void igb_rcv_msg_from_vf(struct igb_adapter *adapter, u32 vf) igb_unlock_mbx(hw, vf); } -static void igb_msg_task(struct igb_adapter *adapter) +/* + * Note: the split of irq and preempible safe parts of igb_msg_task() + * only makes sense under PREEMPT_RT. + * The root cause of igb_rcv_msg_from_vf() is not IRQ safe is because + * it calls kcalloc with GFP_ATOMIC, but GFP_ATOMIC is not IRQ safe + * in PREEMPT_RT. + */ +static void igb_msg_task_irq_safe(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; unsigned long flags; u32 vf; - vfs_lock_irqsave(adapter, &flags); + vfs_raw_spin_lock_irqsave(adapter, &flags); for (vf = 0; vf < adapter->vfs_allocated_count; vf++) { /* process any reset requests */ if (!igb_check_for_rst(hw, vf)) igb_vf_reset_event(adapter, vf); - /* process any messages pending */ - if (!igb_check_for_msg(hw, vf)) - igb_rcv_msg_from_vf(adapter, vf); - /* process any acks */ if (!igb_check_for_ack(hw, vf)) igb_rcv_ack_from_vf(adapter, vf); } - vfs_unlock_irqrestore(adapter, flags); + vfs_raw_spin_unlock_irqrestore(adapter, flags); +} + +static void igb_msg_task_preemptible_safe(struct igb_adapter *adapter) +{ + struct e1000_hw *hw = &adapter->hw; + unsigned long flags; + u32 vf; + + vfs_spin_lock_irqsave(adapter, &flags); + for (vf = 0; vf < adapter->vfs_allocated_count; vf++) { + /* process any messages pending */ + if (!igb_check_for_msg(hw, vf)) + igb_rcv_msg_from_vf(adapter, vf); + } + vfs_spin_unlock_irqrestore(adapter, flags); +} + +static __always_inline void igb_msg_task(struct igb_adapter *adapter) +{ + igb_msg_task_irq_safe(adapter); + igb_msg_task_preemptible_safe(adapter); } /** From patchwork Tue Feb 4 17:52:40 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tony Nguyen X-Patchwork-Id: 13959539 X-Patchwork-Delegate: kuba@kernel.org Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) (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 D11012165F7 for ; Tue, 4 Feb 2025 17:52:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691578; cv=none; b=iFGVIB2pgJRCpg/bHz2GdCAD2+Xdw0AKz/BzNx1z4aNT54TLDMqY+RJWZwpimyTgirIm0q+39XatPEMbXfaX8UZl1AIRmtnS7JJDuZxbjRQBWPO7q9O5rdbncmioredyQSHLJPP1C0CMF4hf5O++kM+KoeLI1HiXG+zX8o+42MY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738691578; c=relaxed/simple; bh=2QsB+IUnFx+X0yxAJ3ioapItqWtm0WFT+lDWHUDiofE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=csf7kcCSaT6nMsy0pqCGS+JbQ+Y6qsPnu5eP5hpWQJfgP/yMCokDhcDPjiCFEyTU3HjsJ+qQnmqTpOZW81jQgdFuMq8xgOrfStOxKjGdZqlt7J0mKhwpRLawmiUCHCCqVmI1nmKQTXKXIBrHMT/HmoJfID7T39zzoowDKFdkugo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=LTKQvEYn; arc=none smtp.client-ip=198.175.65.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="LTKQvEYn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1738691577; x=1770227577; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=2QsB+IUnFx+X0yxAJ3ioapItqWtm0WFT+lDWHUDiofE=; b=LTKQvEYnV4ZoTf/4TWdW3oWQGSR8WWqbkohbFnOY0OEUKFMU8CI99yz/ jDMncYPbFsb6NPnpVSO/kDrbj4EUZa1/DhsZo2YHo3VLMwlHmiPIcWHJl SjczbjNEyhWPzQxj44jzbnErNFqtLjAIx65OOueSl2o5RXOc0bNlAx4Iy pQ3QB+OZc/Zxs7LNhX5MTYiwLkwEJBuz4tQ8GzdO72ApM0SblvIIGvr7e VGAUvYd2JJOrBhaX1PHIfg1UEAiDO2hxzz49O2bw/Ceq3a7xa8E7NGaDa ad+ar03NjVizcMaG/TZVmem0P7GcxKP+IucUSkMf3dVJatTyyJYCEKq+T g==; X-CSE-ConnectionGUID: QdzCytRiQVG2Tnk+qQOYIg== X-CSE-MsgGUID: i7MabNnCRpqwSutYHaaxNQ== X-IronPort-AV: E=McAfee;i="6700,10204,11336"; a="39371905" X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="39371905" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Feb 2025 09:52:54 -0800 X-CSE-ConnectionGUID: dWtmlRkMRlCBjsf+tvueSA== X-CSE-MsgGUID: kb3uPnDsQC6DK2z4SvZgug== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.13,259,1732608000"; d="scan'208";a="110652399" Received: from anguy11-upstream.jf.intel.com ([10.166.9.133]) by orviesa006.jf.intel.com with ESMTP; 04 Feb 2025 09:52:53 -0800 From: Tony Nguyen To: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, edumazet@google.com, andrew+netdev@lunn.ch, netdev@vger.kernel.org Cc: Wander Lairson Costa , anthony.l.nguyen@intel.com, rostedt@goodmis.org, clrkwllms@kernel.org, bigeasy@linutronix.de, jgarzik@redhat.com, yuma@redhat.com, linux-rt-devel@lists.linux.dev, Rafal Romanowski Subject: [PATCH net 4/4] igb: fix igb_msix_other() handling for PREEMPT_RT Date: Tue, 4 Feb 2025 09:52:40 -0800 Message-ID: <20250204175243.810189-5-anthony.l.nguyen@intel.com> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20250204175243.810189-1-anthony.l.nguyen@intel.com> References: <20250204175243.810189-1-anthony.l.nguyen@intel.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org From: Wander Lairson Costa During testing of SR-IOV, Red Hat QE encountered an issue where the ip link up command intermittently fails for the igbvf interfaces when using the PREEMPT_RT variant. Investigation revealed that e1000_write_posted_mbx returns an error due to the lack of an ACK from e1000_poll_for_ack. The underlying issue arises from the fact that IRQs are threaded by default under PREEMPT_RT. While the exact hardware details are not available, it appears that the IRQ handled by igb_msix_other must be processed before e1000_poll_for_ack times out. However, e1000_write_posted_mbx is called with preemption disabled, leading to a scenario where the IRQ is serviced only after the failure of e1000_write_posted_mbx. Commit 338c4d3902fe ("igb: Disable threaded IRQ for igb_msix_other") forced the ISR to run in a non-threaded context. However, Sebastian observed that some functions called within the ISR acquire locks that may sleep. In the previous two patches, we managed to make igb_msg_mask() safe to call from an interrupt context. In this commit, we move most of the ISR handling to an interrupt context, leaving non IRQ safe code to be called from the thread context under PREEMPT_RT. Reproducer: ipaddr_vlan=3 nic_test=ens14f0 vf=${nic_test}v0 # The main testing steps: while true; do ip link set ${nic_test} mtu 1500 ip link set ${vf} mtu 1500 ip link set $vf up # 3. set vlan and ip for VF ip link set ${nic_test} vf 0 vlan ${ipaddr_vlan} ip addr add 172.30.${ipaddr_vlan}.1/24 dev ${vf} ip addr add 2021:db8:${ipaddr_vlan}::1/64 dev ${vf} # 4. check the link state for VF and PF ip link show ${nic_test} if ! ip link show $vf | grep 'state UP'; then echo 'Error found' break fi ip link set $vf down done You can also reproduce it more reliably by setting nr_cpus=1 in the kernel command line. Fixes: 9d5c824399de ("igb: PCI-Express 82575 Gigabit Ethernet driver") Signed-off-by: Wander Lairson Costa Reported-by: Yuying Ma Tested-by: Rafal Romanowski Signed-off-by: Tony Nguyen --- drivers/net/ethernet/intel/igb/igb_main.c | 35 ++++++++++++++++------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 6d590192c27f..3cc85584d9ce 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -128,6 +128,7 @@ static void igb_set_uta(struct igb_adapter *adapter, bool set); static irqreturn_t igb_intr(int irq, void *); static irqreturn_t igb_intr_msi(int irq, void *); static irqreturn_t igb_msix_other(int irq, void *); +static irqreturn_t igb_msix_other_threaded(int irq, void *); static irqreturn_t igb_msix_ring(int irq, void *); #ifdef CONFIG_IGB_DCA static void igb_update_dca(struct igb_q_vector *); @@ -148,7 +149,6 @@ static void igb_rar_set_index(struct igb_adapter *, u32); static void igb_ping_all_vfs(struct igb_adapter *); static void igb_msg_task_irq_safe(struct igb_adapter *adapter); static void igb_msg_task_preemptible_safe(struct igb_adapter *adapter); -static void igb_msg_task(struct igb_adapter *); static void igb_vmm_control(struct igb_adapter *); static int igb_set_vf_mac(struct igb_adapter *, int, unsigned char *); static void igb_flush_mac_table(struct igb_adapter *); @@ -914,8 +914,9 @@ static int igb_request_msix(struct igb_adapter *adapter) struct net_device *netdev = adapter->netdev; int i, err = 0, vector = 0, free_vector = 0; - err = request_irq(adapter->msix_entries[vector].vector, - igb_msix_other, 0, netdev->name, adapter); + err = request_threaded_irq(adapter->msix_entries[vector].vector, + igb_msix_other, igb_msix_other_threaded, + IRQF_NO_THREAD, netdev->name, adapter); if (err) goto err_out; @@ -7156,9 +7157,27 @@ static irqreturn_t igb_msix_other(int irq, void *data) igb_check_wvbr(adapter); } - /* Check for a mailbox event */ + /* Check for a mailbox event (interrupt safe part) */ if (icr & E1000_ICR_VMMB) - igb_msg_task(adapter); + igb_msg_task_irq_safe(adapter); + + adapter->test_icr = icr; + + if (!IS_ENABLED(CONFIG_PREEMPT_RT)) + return igb_msix_other_threaded(irq, data); + + return IRQ_WAKE_THREAD; +} + +static irqreturn_t igb_msix_other_threaded(int irq, void *data) +{ + struct igb_adapter *adapter = data; + struct e1000_hw *hw = &adapter->hw; + u32 icr = adapter->test_icr; + + /* Check for a mailbox event (preempible safe part) */ + if (icr & E1000_ICR_VMMB) + igb_msg_task_preemptible_safe(adapter); if (icr & E1000_ICR_LSC) { hw->mac.get_link_status = 1; @@ -8204,12 +8223,6 @@ static void igb_msg_task_preemptible_safe(struct igb_adapter *adapter) vfs_spin_unlock_irqrestore(adapter, flags); } -static __always_inline void igb_msg_task(struct igb_adapter *adapter) -{ - igb_msg_task_irq_safe(adapter); - igb_msg_task_preemptible_safe(adapter); -} - /** * igb_set_uta - Set unicast filter table address * @adapter: board private structure