All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gal Pressman <gal@nvidia.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>, <netdev@vger.kernel.org>
Cc: Aaron Conole <aconole@redhat.com>,
	Eelco Chaudron <echaudro@redhat.com>,
	Ilya Maximets <i.maximets@ovn.org>,
	Simon Horman <horms@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>, <dev@openvswitch.org>,
	<linux-rt-devel@lists.linux.dev>, Gal Pressman <gal@nvidia.com>
Subject: [PATCH net 2/3] Revert "openvswitch: Use nested-BH locking for ovs_pcpu_storage"
Date: Tue, 10 Jun 2025 09:26:30 +0300	[thread overview]
Message-ID: <20250610062631.1645885-3-gal@nvidia.com> (raw)
In-Reply-To: <20250610062631.1645885-1-gal@nvidia.com>

This reverts commit 672318331b44753ab7bd8545558939c38b4c1132.

Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Gal Pressman <gal@nvidia.com>
---
 net/openvswitch/actions.c  | 31 +++++++++++++++++++++++++++++--
 net/openvswitch/datapath.c | 24 ------------------------
 net/openvswitch/datapath.h | 33 ---------------------------------
 3 files changed, 29 insertions(+), 59 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 435725c27a55..7e4a8d41b9ed 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -39,6 +39,15 @@
 #include "flow_netlink.h"
 #include "openvswitch_trace.h"
 
+struct deferred_action {
+	struct sk_buff *skb;
+	const struct nlattr *actions;
+	int actions_len;
+
+	/* Store pkt_key clone when creating deferred action. */
+	struct sw_flow_key pkt_key;
+};
+
 #define MAX_L2_LEN	(VLAN_ETH_HLEN + 3 * MPLS_HLEN)
 struct ovs_frag_data {
 	unsigned long dst;
@@ -55,10 +64,28 @@ struct ovs_frag_data {
 
 static DEFINE_PER_CPU(struct ovs_frag_data, ovs_frag_data_storage);
 
-DEFINE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage) = {
-	.bh_lock = INIT_LOCAL_LOCK(bh_lock),
+#define DEFERRED_ACTION_FIFO_SIZE 10
+#define OVS_RECURSION_LIMIT 5
+#define OVS_DEFERRED_ACTION_THRESHOLD (OVS_RECURSION_LIMIT - 2)
+struct action_fifo {
+	int head;
+	int tail;
+	/* Deferred action fifo queue storage. */
+	struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
 };
 
+struct action_flow_keys {
+	struct sw_flow_key key[OVS_DEFERRED_ACTION_THRESHOLD];
+};
+
+struct ovs_pcpu_storage {
+	struct action_fifo action_fifos;
+	struct action_flow_keys flow_keys;
+	int exec_level;
+};
+
+static DEFINE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage);
+
 /* Make a clone of the 'key', using the pre-allocated percpu 'flow_keys'
  * space. Return NULL if out of key spaces.
  */
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 6a304ae2d959..aaa6277bb49c 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -244,13 +244,11 @@ void ovs_dp_detach_port(struct vport *p)
 /* Must be called with rcu_read_lock. */
 void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
 {
-	struct ovs_pcpu_storage *ovs_pcpu = this_cpu_ptr(&ovs_pcpu_storage);
 	const struct vport *p = OVS_CB(skb)->input_vport;
 	struct datapath *dp = p->dp;
 	struct sw_flow *flow;
 	struct sw_flow_actions *sf_acts;
 	struct dp_stats_percpu *stats;
-	bool ovs_pcpu_locked = false;
 	u64 *stats_counter;
 	u32 n_mask_hit;
 	u32 n_cache_hit;
@@ -292,26 +290,10 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key)
 
 	ovs_flow_stats_update(flow, key->tp.flags, skb);
 	sf_acts = rcu_dereference(flow->sf_acts);
-	/* This path can be invoked recursively: Use the current task to
-	 * identify recursive invocation - the lock must be acquired only once.
-	 * Even with disabled bottom halves this can be preempted on PREEMPT_RT.
-	 * Limit the locking to RT to avoid assigning `owner' if it can be
-	 * avoided.
-	 */
-	if (IS_ENABLED(CONFIG_PREEMPT_RT) && ovs_pcpu->owner != current) {
-		local_lock_nested_bh(&ovs_pcpu_storage.bh_lock);
-		ovs_pcpu->owner = current;
-		ovs_pcpu_locked = true;
-	}
-
 	error = ovs_execute_actions(dp, skb, sf_acts, key);
 	if (unlikely(error))
 		net_dbg_ratelimited("ovs: action execution error on datapath %s: %d\n",
 				    ovs_dp_name(dp), error);
-	if (ovs_pcpu_locked) {
-		ovs_pcpu->owner = NULL;
-		local_unlock_nested_bh(&ovs_pcpu_storage.bh_lock);
-	}
 
 	stats_counter = &stats->n_hit;
 
@@ -689,13 +671,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	sf_acts = rcu_dereference(flow->sf_acts);
 
 	local_bh_disable();
-	local_lock_nested_bh(&ovs_pcpu_storage.bh_lock);
-	if (IS_ENABLED(CONFIG_PREEMPT_RT))
-		this_cpu_write(ovs_pcpu_storage.owner, current);
 	err = ovs_execute_actions(dp, packet, sf_acts, &flow->key);
-	if (IS_ENABLED(CONFIG_PREEMPT_RT))
-		this_cpu_write(ovs_pcpu_storage.owner, NULL);
-	local_unlock_nested_bh(&ovs_pcpu_storage.bh_lock);
 	local_bh_enable();
 	rcu_read_unlock();
 
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 4a665c3cfa90..a12640792605 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -173,39 +173,6 @@ struct ovs_net {
 	bool xt_label;
 };
 
-struct deferred_action {
-	struct sk_buff *skb;
-	const struct nlattr *actions;
-	int actions_len;
-
-	/* Store pkt_key clone when creating deferred action. */
-	struct sw_flow_key pkt_key;
-};
-
-#define DEFERRED_ACTION_FIFO_SIZE 10
-#define OVS_RECURSION_LIMIT 5
-#define OVS_DEFERRED_ACTION_THRESHOLD (OVS_RECURSION_LIMIT - 2)
-
-struct action_fifo {
-	int head;
-	int tail;
-	/* Deferred action fifo queue storage. */
-	struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
-};
-
-struct action_flow_keys {
-	struct sw_flow_key key[OVS_DEFERRED_ACTION_THRESHOLD];
-};
-
-struct ovs_pcpu_storage {
-	struct action_fifo action_fifos;
-	struct action_flow_keys flow_keys;
-	int exec_level;
-	struct task_struct *owner;
-	local_lock_t bh_lock;
-};
-DECLARE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage);
-
 /**
  * enum ovs_pkt_hash_types - hash info to include with a packet
  * to send to userspace.
-- 
2.40.1


  parent reply	other threads:[~2025-06-10  6:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-10  6:26 [PATCH net 0/3] Revert openvswitch per-CPU storage Gal Pressman
2025-06-10  6:26 ` [PATCH net 1/3] Revert "openvswitch: Move ovs_frag_data_storage into the struct ovs_pcpu_storage" Gal Pressman
2025-06-10  6:26 ` Gal Pressman [this message]
2025-06-10  6:26 ` [PATCH net 3/3] Revert "openvswitch: Merge three per-CPU structures into one" Gal Pressman
2025-06-10  6:43 ` [PATCH net 0/3] Revert openvswitch per-CPU storage Sebastian Andrzej Siewior
2025-06-10  7:03   ` Gal Pressman
2025-06-10  7:06     ` Sebastian Andrzej Siewior
2025-06-10  7:08       ` Gal Pressman
2025-06-11  6:59         ` Sebastian Andrzej Siewior
2025-06-11 11:56           ` Gal Pressman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250610062631.1645885-3-gal@nvidia.com \
    --to=gal@nvidia.com \
    --cc=aconole@redhat.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dev@openvswitch.org \
    --cc=echaudro@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=i.maximets@ovn.org \
    --cc=kuba@kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.