* [patch net-next 0/2] team: two RCU fixups
@ 2012-06-20 15:31 Jiri Pirko
2012-06-20 15:32 ` [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer Jiri Pirko
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jiri Pirko @ 2012-06-20 15:31 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, jbrouer, paulmck, wfg
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer
2012-06-20 15:31 [patch net-next 0/2] team: two RCU fixups Jiri Pirko
@ 2012-06-20 15:32 ` Jiri Pirko
2012-06-20 16:01 ` Eric Dumazet
2012-06-20 15:32 ` [patch net-next 2/2] team: use RCU_INIT_POINTER for NULL assignment of RCU pointer Jiri Pirko
2012-06-20 21:05 ` [patch net-next 0/2] team: two RCU fixups David Miller
2 siblings, 1 reply; 8+ messages in thread
From: Jiri Pirko @ 2012-06-20 15:32 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, jbrouer, paulmck, wfg
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [patch net-next 2/2] team: use RCU_INIT_POINTER for NULL assignment of RCU pointer
2012-06-20 15:31 [patch net-next 0/2] team: two RCU fixups Jiri Pirko
2012-06-20 15:32 ` [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer Jiri Pirko
@ 2012-06-20 15:32 ` Jiri Pirko
2012-06-20 21:05 ` [patch net-next 0/2] team: two RCU fixups David Miller
2 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2012-06-20 15:32 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, jbrouer, paulmck, wfg
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer
2012-06-20 15:32 ` [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer Jiri Pirko
@ 2012-06-20 16:01 ` Eric Dumazet
2012-06-20 18:33 ` Jiri Pirko
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-06-20 16:01 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, jbrouer, paulmck, wfg
On Wed, 2012-06-20 at 17:32 +0200, Jiri Pirko wrote:
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> 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));
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer
2012-06-20 16:01 ` Eric Dumazet
@ 2012-06-20 18:33 ` Jiri Pirko
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Pirko @ 2012-06-20 18:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem, jbrouer, paulmck, wfg
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:
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>> 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.
>
>> + 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...
Squashing into one now.
>
>> 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));
>> }
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch net-next 0/2] team: two RCU fixups
2012-06-20 15:31 [patch net-next 0/2] team: two RCU fixups Jiri Pirko
2012-06-20 15:32 ` [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer Jiri Pirko
2012-06-20 15:32 ` [patch net-next 2/2] team: use RCU_INIT_POINTER for NULL assignment of RCU pointer Jiri Pirko
@ 2012-06-20 21:05 ` David Miller
2012-06-20 21:19 ` Eric Dumazet
2 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2012-06-20 21:05 UTC (permalink / raw)
To: jpirko; +Cc: netdev, eric.dumazet, jbrouer, paulmck, wfg
From: Jiri Pirko <jpirko@redhat.com>
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch net-next 0/2] team: two RCU fixups
2012-06-20 21:05 ` [patch net-next 0/2] team: two RCU fixups David Miller
@ 2012-06-20 21:19 ` Eric Dumazet
2012-06-20 21:27 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2012-06-20 21:19 UTC (permalink / raw)
To: David Miller; +Cc: jpirko, netdev, jbrouer, paulmck, wfg
On Wed, 2012-06-20 at 14:05 -0700, David Miller wrote:
> From: Jiri Pirko <jpirko@redhat.com>
> 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.
I reviewed them and spotted problems, and you applied them...
Then Jiri sent an update.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch net-next 0/2] team: two RCU fixups
2012-06-20 21:19 ` Eric Dumazet
@ 2012-06-20 21:27 ` David Miller
0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2012-06-20 21:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: jpirko, netdev, jbrouer, paulmck, wfg
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 20 Jun 2012 23:19:36 +0200
> On Wed, 2012-06-20 at 14:05 -0700, David Miller wrote:
>> From: Jiri Pirko <jpirko@redhat.com>
>> 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.
>
> I reviewed them and spotted problems, and you applied them...
>
> Then Jiri sent an update.
Sorry, I'll fix this up.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-06-20 21:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-20 15:31 [patch net-next 0/2] team: two RCU fixups Jiri Pirko
2012-06-20 15:32 ` [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer Jiri Pirko
2012-06-20 16:01 ` Eric Dumazet
2012-06-20 18:33 ` Jiri Pirko
2012-06-20 15:32 ` [patch net-next 2/2] team: use RCU_INIT_POINTER for NULL assignment of RCU pointer Jiri Pirko
2012-06-20 21:05 ` [patch net-next 0/2] team: two RCU fixups David Miller
2012-06-20 21:19 ` Eric Dumazet
2012-06-20 21:27 ` David Miller
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.