Message ID | 20200224093013.25700-1-madhuparnabhowmik10@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | net: core: devlink.c: Use built-in RCU list checking | expand |
Mon, Feb 24, 2020 at 10:30:13AM CET, madhuparnabhowmik10@gmail.com wrote: >From: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> > >list_for_each_entry_rcu() has built-in RCU and lock checking. > >Pass cond argument to list_for_each_entry_rcu() to silence >false lockdep warning when CONFIG_PROVE_RCU_LIST is enabled. > >The devlink->lock is held when devlink_dpipe_table_find() >is called in non RCU read side section. Therefore, pass struct devlink >to devlink_dpipe_table_find() for lockdep checking. > >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> >--- > net/core/devlink.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > >diff --git a/net/core/devlink.c b/net/core/devlink.c >index e82750bdc496..dadf5fa79bb1 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -2103,11 +2103,11 @@ static int devlink_dpipe_entry_put(struct sk_buff *skb, > > static struct devlink_dpipe_table * > devlink_dpipe_table_find(struct list_head *dpipe_tables, >- const char *table_name) >+ const char *table_name, struct devlink *devlink) > { > struct devlink_dpipe_table *table; >- >- list_for_each_entry_rcu(table, dpipe_tables, list) { >+ list_for_each_entry_rcu(table, dpipe_tables, list, >+ lockdep_is_held(&devlink->lock)) { > if (!strcmp(table->name, table_name)) > return table; > } >@@ -2226,7 +2226,7 @@ static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb, > > table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]); > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, >- table_name); >+ table_name, devlink); > if (!table) > return -EINVAL; > >@@ -2382,7 +2382,7 @@ static int devlink_dpipe_table_counters_set(struct devlink *devlink, > struct devlink_dpipe_table *table; > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, >- table_name); >+ table_name, devlink); > if (!table) > return -EINVAL; > >@@ -6814,7 +6814,7 @@ bool devlink_dpipe_table_counter_enabled(struct devlink *devlink, > > rcu_read_lock(); > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, >- table_name); >+ table_name, devlink); > enabled = false; > if (table) > enabled = table->counters_enabled; >@@ -6845,7 +6845,7 @@ int devlink_dpipe_table_register(struct devlink *devlink, > > mutex_lock(&devlink->lock); > >- if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name)) { >+ if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name, devlink)) { Run scripts/checkpatch.pl on your patch. You are breaking 80-cols limit here. Otherwise, the patch looks fine. > err = -EEXIST; > goto unlock; > } >@@ -6881,7 +6881,7 @@ void devlink_dpipe_table_unregister(struct devlink *devlink, > > mutex_lock(&devlink->lock); > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, >- table_name); >+ table_name, devlink); > if (!table) > goto unlock; > list_del_rcu(&table->list); >@@ -7038,7 +7038,7 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink, > > mutex_lock(&devlink->lock); > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, >- table_name); >+ table_name, devlink); > if (!table) { > err = -EINVAL; > goto out; >-- >2.17.1 >
On Mon, Feb 24, 2020 at 11:52:09AM +0100, Jiri Pirko wrote: > Mon, Feb 24, 2020 at 10:30:13AM CET, madhuparnabhowmik10@gmail.com wrote: > >From: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> > > > >list_for_each_entry_rcu() has built-in RCU and lock checking. > > > >Pass cond argument to list_for_each_entry_rcu() to silence > >false lockdep warning when CONFIG_PROVE_RCU_LIST is enabled. > > > >The devlink->lock is held when devlink_dpipe_table_find() > >is called in non RCU read side section. Therefore, pass struct devlink > >to devlink_dpipe_table_find() for lockdep checking. > > > >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> > >--- > > net/core/devlink.c | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > >diff --git a/net/core/devlink.c b/net/core/devlink.c > >index e82750bdc496..dadf5fa79bb1 100644 > >--- a/net/core/devlink.c > >+++ b/net/core/devlink.c > >@@ -2103,11 +2103,11 @@ static int devlink_dpipe_entry_put(struct sk_buff *skb, > > > > static struct devlink_dpipe_table * > > devlink_dpipe_table_find(struct list_head *dpipe_tables, > >- const char *table_name) > >+ const char *table_name, struct devlink *devlink) > > { > > struct devlink_dpipe_table *table; > >- > >- list_for_each_entry_rcu(table, dpipe_tables, list) { > >+ list_for_each_entry_rcu(table, dpipe_tables, list, > >+ lockdep_is_held(&devlink->lock)) { > > if (!strcmp(table->name, table_name)) > > return table; > > } > >@@ -2226,7 +2226,7 @@ static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb, > > > > table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]); > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, > >- table_name); > >+ table_name, devlink); > > if (!table) > > return -EINVAL; > > > >@@ -2382,7 +2382,7 @@ static int devlink_dpipe_table_counters_set(struct devlink *devlink, > > struct devlink_dpipe_table *table; > > > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, > >- table_name); > >+ table_name, devlink); > > if (!table) > > return -EINVAL; > > > >@@ -6814,7 +6814,7 @@ bool devlink_dpipe_table_counter_enabled(struct devlink *devlink, > > > > rcu_read_lock(); > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, > >- table_name); > >+ table_name, devlink); > > enabled = false; > > if (table) > > enabled = table->counters_enabled; > >@@ -6845,7 +6845,7 @@ int devlink_dpipe_table_register(struct devlink *devlink, > > > > mutex_lock(&devlink->lock); > > > >- if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name)) { > >+ if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name, devlink)) { > > Run scripts/checkpatch.pl on your patch. You are breaking 80-cols limit > here. > Sure, I will take care of this and send the updated patch soon. Thank you, Madhuparna > Otherwise, the patch looks fine. > > > err = -EEXIST; > > goto unlock; > > } > >@@ -6881,7 +6881,7 @@ void devlink_dpipe_table_unregister(struct devlink *devlink, > > > > mutex_lock(&devlink->lock); > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, > >- table_name); > >+ table_name, devlink); > > if (!table) > > goto unlock; > > list_del_rcu(&table->list); > >@@ -7038,7 +7038,7 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink, > > > > mutex_lock(&devlink->lock); > > table = devlink_dpipe_table_find(&devlink->dpipe_table_list, > >- table_name); > >+ table_name, devlink); > > if (!table) { > > err = -EINVAL; > > goto out; > >-- > >2.17.1 > >
diff --git a/net/core/devlink.c b/net/core/devlink.c index e82750bdc496..dadf5fa79bb1 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -2103,11 +2103,11 @@ static int devlink_dpipe_entry_put(struct sk_buff *skb, static struct devlink_dpipe_table * devlink_dpipe_table_find(struct list_head *dpipe_tables, - const char *table_name) + const char *table_name, struct devlink *devlink) { struct devlink_dpipe_table *table; - - list_for_each_entry_rcu(table, dpipe_tables, list) { + list_for_each_entry_rcu(table, dpipe_tables, list, + lockdep_is_held(&devlink->lock)) { if (!strcmp(table->name, table_name)) return table; } @@ -2226,7 +2226,7 @@ static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb, table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]); table = devlink_dpipe_table_find(&devlink->dpipe_table_list, - table_name); + table_name, devlink); if (!table) return -EINVAL; @@ -2382,7 +2382,7 @@ static int devlink_dpipe_table_counters_set(struct devlink *devlink, struct devlink_dpipe_table *table; table = devlink_dpipe_table_find(&devlink->dpipe_table_list, - table_name); + table_name, devlink); if (!table) return -EINVAL; @@ -6814,7 +6814,7 @@ bool devlink_dpipe_table_counter_enabled(struct devlink *devlink, rcu_read_lock(); table = devlink_dpipe_table_find(&devlink->dpipe_table_list, - table_name); + table_name, devlink); enabled = false; if (table) enabled = table->counters_enabled; @@ -6845,7 +6845,7 @@ int devlink_dpipe_table_register(struct devlink *devlink, mutex_lock(&devlink->lock); - if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name)) { + if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name, devlink)) { err = -EEXIST; goto unlock; } @@ -6881,7 +6881,7 @@ void devlink_dpipe_table_unregister(struct devlink *devlink, mutex_lock(&devlink->lock); table = devlink_dpipe_table_find(&devlink->dpipe_table_list, - table_name); + table_name, devlink); if (!table) goto unlock; list_del_rcu(&table->list); @@ -7038,7 +7038,7 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink, mutex_lock(&devlink->lock); table = devlink_dpipe_table_find(&devlink->dpipe_table_list, - table_name); + table_name, devlink); if (!table) { err = -EINVAL; goto out;