8 messages, 3 authors, 2012-06-20 · open the first message on its own page
From: Jiri Pirko <hidden>
Date: 2012-06-20 15:32:07
Jiri Pirko (2): team: use rcu_access_pointer to access RCU pointer by writer team: use RCU_INIT_POINTER for NULL assignment of RCU pointer drivers/net/team/team_mode_activebackup.c | 7 +++++-- drivers/net/team/team_mode_loadbalance.c | 10 ++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) -- 1.7.10.4
From: Jiri Pirko <hidden>
Date: 2012-06-20 15:32:55
Signed-off-by: Jiri Pirko <redacted> --- drivers/net/team/team_mode_loadbalance.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index b4475a5..c385b45 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c@@ -97,7 +97,7 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team, pm = &lb_priv->ex->tx_hash_to_port_mapping[i]; if (rcu_access_pointer(pm->port) == port) { - rcu_assign_pointer(pm->port, NULL); + RCU_INIT_POINTER(pm->port, NULL); team_option_inst_set_change(pm->opt_inst_info); changed = true; }
--
1.7.10.4
From: Jiri Pirko <hidden>
Date: 2012-06-20 15:33:09
Signed-off-by: Jiri Pirko <redacted> --- drivers/net/team/team_mode_activebackup.c | 7 +++++-- drivers/net/team/team_mode_loadbalance.c | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index 2fe02a8..c9e7621 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c@@ -61,8 +61,11 @@ static void ab_port_leave(struct team *team, struct team_port *port) static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx) { - if (ab_priv(team)->active_port) - ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex; + struct team_port *active_port; + + active_port = rcu_access_pointer(ab_priv(team)->active_port); + if (active_port) + ctx->data.u32_val = active_port->dev->ifindex; else ctx->data.u32_val = 0; return 0;
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 45cc095..b4475a5 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c@@ -96,7 +96,7 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team, struct lb_port_mapping *pm; pm = &lb_priv->ex->tx_hash_to_port_mapping[i]; - if (pm->port == port) { + if (rcu_access_pointer(pm->port) == port) { rcu_assign_pointer(pm->port, NULL); team_option_inst_set_change(pm->opt_inst_info); changed = true;
@@ -292,7 +292,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx) if (lb_priv->ex->orig_fprog) { /* Clear old filter data */ __fprog_destroy(lb_priv->ex->orig_fprog); - sk_unattached_filter_destroy(lb_priv->fp); + sk_unattached_filter_destroy(rcu_access_pointer(lb_priv->fp)); } rcu_assign_pointer(lb_priv->fp, fp);
@@ -303,9 +303,11 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx) static int lb_tx_method_get(struct team *team, struct team_gsetter_ctx *ctx) { struct lb_priv *lb_priv = get_lb_priv(team); + lb_select_tx_port_func_t *func; char *name; - name = lb_select_tx_port_get_name(lb_priv->select_tx_port_func); + func = rcu_access_pointer(lb_priv->select_tx_port_func); + name = lb_select_tx_port_get_name(func); BUG_ON(!name); ctx->data.str_val = name; return 0;
--
1.7.10.4
From: Eric Dumazet <hidden>
Date: 2012-06-20 16:01:49
On Wed, 2012-06-20 at 17:32 +0200, Jiri Pirko wrote:
Signed-off-by: Jiri Pirko <redacted> --- drivers/net/team/team_mode_activebackup.c | 7 +++++-- drivers/net/team/team_mode_loadbalance.c | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-)diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c index 2fe02a8..c9e7621 100644 --- a/drivers/net/team/team_mode_activebackup.c +++ b/drivers/net/team/team_mode_activebackup.c@@ -61,8 +61,11 @@ static void ab_port_leave(struct team *team, struct team_port *port) static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx) { - if (ab_priv(team)->active_port) - ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex; + struct team_port *active_port; + + active_port = rcu_access_pointer(ab_priv(team)->active_port);
This is not the correct fix. You cant safely dereference active_port if you got it from rcu_access_pointer() You should use rcu_dereference() of rcu_dereference_protected() or rcu_dereference_bh() or similar variant, depending on the context.
+ if (active_port) + ctx->data.u32_val = active_port->dev->ifindex; else ctx->data.u32_val = 0; return 0;diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c index 45cc095..b4475a5 100644 --- a/drivers/net/team/team_mode_loadbalance.c +++ b/drivers/net/team/team_mode_loadbalance.c@@ -96,7 +96,7 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team, struct lb_port_mapping *pm; pm = &lb_priv->ex->tx_hash_to_port_mapping[i]; - if (pm->port == port) { + if (rcu_access_pointer(pm->port) == port) {
This one is OK
rcu_assign_pointer(pm->port, NULL);
I dont understand why you submit two patches...
team_option_inst_set_change(pm->opt_inst_info); changed = true;@@ -292,7 +292,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx) if (lb_priv->ex->orig_fprog) { /* Clear old filter data */ __fprog_destroy(lb_priv->ex->orig_fprog); - sk_unattached_filter_destroy(lb_priv->fp); + sk_unattached_filter_destroy(rcu_access_pointer(lb_priv->fp)); }
From: Jiri Pirko <hidden>
Date: 2012-06-20 18:34:08
Wed, Jun 20, 2012 at 06:01:44PM CEST, eric.dumazet@gmail.com wrote:
On Wed, 2012-06-20 at 17:32 +0200, Jiri Pirko wrote:quoted
Signed-off-by: Jiri Pirko <redacted> --- drivers/net/team/team_mode_activebackup.c | 7 +++++-- drivers/net/team/team_mode_loadbalance.c | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-)diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c index 2fe02a8..c9e7621 100644 --- a/drivers/net/team/team_mode_activebackup.c +++ b/drivers/net/team/team_mode_activebackup.c@@ -61,8 +61,11 @@ static void ab_port_leave(struct team *team, struct team_port *port) static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx) { - if (ab_priv(team)->active_port) - ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex; + struct team_port *active_port; + + active_port = rcu_access_pointer(ab_priv(team)->active_port);This is not the correct fix. You cant safely dereference active_port if you got it from rcu_access_pointer()
You should use rcu_dereference() of rcu_dereference_protected() or rcu_dereference_bh() or similar variant, depending on the context.
Okay, reworking this using rcu_dereference_protected() since this is update path.
quoted
+ if (active_port) + ctx->data.u32_val = active_port->dev->ifindex; else ctx->data.u32_val = 0; return 0;diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c index 45cc095..b4475a5 100644 --- a/drivers/net/team/team_mode_loadbalance.c +++ b/drivers/net/team/team_mode_loadbalance.c@@ -96,7 +96,7 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team, struct lb_port_mapping *pm; pm = &lb_priv->ex->tx_hash_to_port_mapping[i]; - if (pm->port == port) { + if (rcu_access_pointer(pm->port) == port) {This one is OKquoted
rcu_assign_pointer(pm->port, NULL);I dont understand why you submit two patches...
Squashing into one now.
quoted
team_option_inst_set_change(pm->opt_inst_info); changed = true;@@ -292,7 +292,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx) if (lb_priv->ex->orig_fprog) { /* Clear old filter data */ __fprog_destroy(lb_priv->ex->orig_fprog); - sk_unattached_filter_destroy(lb_priv->fp); + sk_unattached_filter_destroy(rcu_access_pointer(lb_priv->fp)); }
From: David Miller <davem@davemloft.net>
Date: 2012-06-20 21:05:17
From: Jiri Pirko <redacted> Date: Wed, 20 Jun 2012 17:31:59 +0200
Jiri Pirko (2): team: use rcu_access_pointer to access RCU pointer by writer team: use RCU_INIT_POINTER for NULL assignment of RCU pointer
Applied, but this makes your subsequent patch not apply.
From: Eric Dumazet <hidden>
Date: 2012-06-20 21:19:41
On Wed, 2012-06-20 at 14:05 -0700, David Miller wrote:
From: Jiri Pirko <redacted> Date: Wed, 20 Jun 2012 17:31:59 +0200quoted
Jiri Pirko (2): team: use rcu_access_pointer to access RCU pointer by writer team: use RCU_INIT_POINTER for NULL assignment of RCU pointerApplied, but this makes your subsequent patch not apply.
I reviewed them and spotted problems, and you applied them... Then Jiri sent an update.
From: David Miller <davem@davemloft.net>
Date: 2012-06-20 21:27:37
From: Eric Dumazet <redacted> Date: Wed, 20 Jun 2012 23:19:36 +0200
On Wed, 2012-06-20 at 14:05 -0700, David Miller wrote:quoted
From: Jiri Pirko <redacted> Date: Wed, 20 Jun 2012 17:31:59 +0200quoted
Jiri Pirko (2): team: use rcu_access_pointer to access RCU pointer by writer team: use RCU_INIT_POINTER for NULL assignment of RCU pointerApplied, but this makes your subsequent patch not apply.I reviewed them and spotted problems, and you applied them... Then Jiri sent an update.
Sorry, I'll fix this up.