* [PATCH] net: core: devlink.c: Use built-in RCU list checking
@ 2020-02-24 9:30 madhuparnabhowmik10
2020-02-24 10:52 ` Jiri Pirko
0 siblings, 1 reply; 14+ messages in thread
From: madhuparnabhowmik10 @ 2020-02-24 9:30 UTC (permalink / raw)
To: jiri, davem
Cc: netdev, linux-kernel, joel, linux-kernel-mentees, frextrite,
paulmck, Madhuparna Bhowmik
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)) {
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-24 9:30 [PATCH] net: core: devlink.c: Use built-in RCU list checking madhuparnabhowmik10
@ 2020-02-24 10:52 ` Jiri Pirko
2020-02-25 12:09 ` Madhuparna Bhowmik
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2020-02-24 10:52 UTC (permalink / raw)
To: madhuparnabhowmik10
Cc: jiri, davem, netdev, linux-kernel, joel, linux-kernel-mentees,
frextrite, paulmck
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-24 10:52 ` Jiri Pirko
@ 2020-02-25 12:09 ` Madhuparna Bhowmik
0 siblings, 0 replies; 14+ messages in thread
From: Madhuparna Bhowmik @ 2020-02-25 12:09 UTC (permalink / raw)
To: Jiri Pirko
Cc: madhuparnabhowmik10, jiri, davem, netdev, linux-kernel, joel,
linux-kernel-mentees, frextrite, paulmck
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
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-25 12:27 madhuparnabhowmik10
2020-02-25 13:06 ` Jiri Pirko
@ 2020-02-27 0:59 ` David Miller
1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2020-02-27 0:59 UTC (permalink / raw)
To: madhuparnabhowmik10
Cc: jiri, netdev, linux-kernel, joel, linux-kernel-mentees,
frextrite, paulmck
From: madhuparnabhowmik10@gmail.com
Date: Tue, 25 Feb 2020 17:57:45 +0530
> 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>
Applied, thank you.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-25 12:27 madhuparnabhowmik10
@ 2020-02-25 13:06 ` Jiri Pirko
2020-02-27 0:59 ` David Miller
1 sibling, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2020-02-25 13:06 UTC (permalink / raw)
To: madhuparnabhowmik10
Cc: jiri, davem, netdev, linux-kernel, joel, linux-kernel-mentees,
frextrite, paulmck
Tue, Feb 25, 2020 at 01:27:45PM 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>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] net: core: devlink.c: Use built-in RCU list checking
@ 2020-02-25 12:27 madhuparnabhowmik10
2020-02-25 13:06 ` Jiri Pirko
2020-02-27 0:59 ` David Miller
0 siblings, 2 replies; 14+ messages in thread
From: madhuparnabhowmik10 @ 2020-02-25 12:27 UTC (permalink / raw)
To: jiri, davem
Cc: netdev, linux-kernel, joel, linux-kernel-mentees, frextrite,
paulmck, Madhuparna Bhowmik
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 | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e82750bdc496..a92ab58304e5 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,8 @@ 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 +6882,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 +7039,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 16:51 madhuparnabhowmik10
2020-02-21 17:20 ` Jiri Pirko
@ 2020-02-24 0:28 ` kbuild test robot
1 sibling, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2020-02-24 0:28 UTC (permalink / raw)
To: madhuparnabhowmik10
Cc: kbuild-all, clang-built-linux, jiri, davem, netdev, linux-kernel,
joel, frextrite, linux-kernel-mentees, paulmck,
Madhuparna Bhowmik
[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on net/master linus/master v5.6-rc2 next-20200221]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/madhuparnabhowmik10-gmail-com/net-core-devlink-c-Use-built-in-RCU-list-checking/20200223-035655
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 732a0dee501f9a693c9a711730838129f4587041
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (git://gitmirror/llvm_project 1df947ab403a9ec3bb1bf4cd83610a997dc4f3bc)
reproduce:
# FIXME the reproduce steps for clang is not ready yet
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> net/core/devlink.c:2111:22: error: use of undeclared identifier 'devlink'
lockdep_is_held(&devlink->lock)) {
^
1 error generated.
vim +/devlink +2111 net/core/devlink.c
2103
2104 static struct devlink_dpipe_table *
2105 devlink_dpipe_table_find(struct list_head *dpipe_tables,
2106 const char *table_name)
2107 {
2108 struct devlink_dpipe_table *table;
2109
2110 list_for_each_entry_rcu(table, dpipe_tables, list,
> 2111 lockdep_is_held(&devlink->lock)) {
2112 if (!strcmp(table->name, table_name))
2113 return table;
2114 }
2115 return NULL;
2116 }
2117
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72192 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-23 11:03 ` Madhuparna Bhowmik
@ 2020-02-23 18:19 ` Jiri Pirko
0 siblings, 0 replies; 14+ messages in thread
From: Jiri Pirko @ 2020-02-23 18:19 UTC (permalink / raw)
To: Madhuparna Bhowmik
Cc: jiri, davem, netdev, linux-kernel, joel, frextrite,
linux-kernel-mentees, paulmck
Sun, Feb 23, 2020 at 12:03:42PM CET, madhuparnabhowmik10@gmail.com wrote:
>On Fri, Feb 21, 2020 at 06:20:08PM +0100, Jiri Pirko wrote:
>> Fri, Feb 21, 2020 at 05:51:41PM 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
>> >by default.
>> >
>> >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
>>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>>
>> Thanks.
>>
>> However, there is a callpath where not devlink lock neither rcu read is
>> taken:
>> devlink_dpipe_table_register()->devlink_dpipe_table_find()
>> I guess that was not the trace you were seeing, right?
>>
>>
>> >---
>> > net/core/devlink.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/net/core/devlink.c b/net/core/devlink.c
>> >index 4c63c9a4c09e..3e8c94155d93 100644
>> >--- a/net/core/devlink.c
>> >+++ b/net/core/devlink.c
>> >@@ -2107,7 +2107,8 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
>> > {
>> > 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)) {
>
>Hi Jiri,
>
>I just noticed that this patch does not compile because devlink is
>not passed as an argument to devlink_dpipe_table_find() and it is not
>even global. I am not sure why I didn't encounter this error before
>sending the patch. Anyway, I am sorry about this.
>But it seems to be the right lock that should be held and checked for
>in devlink_dpipe_table_find().
>So will it be okay to pass devlink to devlink_dpipe_table_find()?
Sure.
>Anyway devlink_dpipe_table_find() is only called from functions
>within devlink.c.
>
>Let me know what you think about this.
>Thank you,
>Madhuparna
>
>
>> > if (!strcmp(table->name, table_name))
>> > return table;
>> > }
>> >--
>> >2.17.1
>> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 17:20 ` Jiri Pirko
2020-02-21 17:35 ` Madhuparna Bhowmik
@ 2020-02-23 11:03 ` Madhuparna Bhowmik
2020-02-23 18:19 ` Jiri Pirko
1 sibling, 1 reply; 14+ messages in thread
From: Madhuparna Bhowmik @ 2020-02-23 11:03 UTC (permalink / raw)
To: Jiri Pirko
Cc: madhuparnabhowmik10, jiri, davem, netdev, linux-kernel, joel,
frextrite, linux-kernel-mentees, paulmck
On Fri, Feb 21, 2020 at 06:20:08PM +0100, Jiri Pirko wrote:
> Fri, Feb 21, 2020 at 05:51:41PM 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
> >by default.
> >
> >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>
> Thanks.
>
> However, there is a callpath where not devlink lock neither rcu read is
> taken:
> devlink_dpipe_table_register()->devlink_dpipe_table_find()
> I guess that was not the trace you were seeing, right?
>
>
> >---
> > net/core/devlink.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/net/core/devlink.c b/net/core/devlink.c
> >index 4c63c9a4c09e..3e8c94155d93 100644
> >--- a/net/core/devlink.c
> >+++ b/net/core/devlink.c
> >@@ -2107,7 +2107,8 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
> > {
> > 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)) {
Hi Jiri,
I just noticed that this patch does not compile because devlink is
not passed as an argument to devlink_dpipe_table_find() and it is not
even global. I am not sure why I didn't encounter this error before
sending the patch. Anyway, I am sorry about this.
But it seems to be the right lock that should be held and checked for
in devlink_dpipe_table_find().
So will it be okay to pass devlink to devlink_dpipe_table_find()?
Anyway devlink_dpipe_table_find() is only called from functions
within devlink.c.
Let me know what you think about this.
Thank you,
Madhuparna
> > if (!strcmp(table->name, table_name))
> > return table;
> > }
> >--
> >2.17.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 17:54 ` Jiri Pirko
@ 2020-02-21 18:00 ` Madhuparna Bhowmik
0 siblings, 0 replies; 14+ messages in thread
From: Madhuparna Bhowmik @ 2020-02-21 18:00 UTC (permalink / raw)
To: Jiri Pirko, y
Cc: Madhuparna Bhowmik, jiri, davem, netdev, linux-kernel, joel,
frextrite, linux-kernel-mentees, paulmck
On Fri, Feb 21, 2020 at 06:54:36PM +0100, Jiri Pirko wrote:
> Fri, Feb 21, 2020 at 06:35:34PM CET, madhuparnabhowmik10@gmail.com wrote:
> >On Fri, Feb 21, 2020 at 06:20:08PM +0100, Jiri Pirko wrote:
> >> Fri, Feb 21, 2020 at 05:51:41PM 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
> >> >by default.
> >> >
> >> >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
> >>
> >> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> >>
> >> Thanks.
> >>
> >> However, there is a callpath where not devlink lock neither rcu read is
> >> taken:
> >> devlink_dpipe_table_register()->devlink_dpipe_table_find()
> >>
> >Hi,
> >
> >Yes I had noticed this, but I was not sure if there is some other lock
> >which is being used.
> >
> >If yes, then can you please tell me which lock is held in this case,
> >and I can add that condition as well to list_for_each_entry_rcu() usage.
> >
> >And if no lock or rcu_read_lock is held then may be we should
> >use rcu_read_lock/unlock here.
> >
> >Let me know what you think about this.
>
> devlink->lock should be held since the beginning of
> devlink_dpipe_table_register()
>
Alright, I will send a patch with this change soon.
Thank you for the help.
Regards,
Madhuparna
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 17:35 ` Madhuparna Bhowmik
@ 2020-02-21 17:54 ` Jiri Pirko
2020-02-21 18:00 ` Madhuparna Bhowmik
0 siblings, 1 reply; 14+ messages in thread
From: Jiri Pirko @ 2020-02-21 17:54 UTC (permalink / raw)
To: Madhuparna Bhowmik
Cc: jiri, davem, netdev, linux-kernel, joel, frextrite,
linux-kernel-mentees, paulmck
Fri, Feb 21, 2020 at 06:35:34PM CET, madhuparnabhowmik10@gmail.com wrote:
>On Fri, Feb 21, 2020 at 06:20:08PM +0100, Jiri Pirko wrote:
>> Fri, Feb 21, 2020 at 05:51:41PM 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
>> >by default.
>> >
>> >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
>>
>> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>>
>> Thanks.
>>
>> However, there is a callpath where not devlink lock neither rcu read is
>> taken:
>> devlink_dpipe_table_register()->devlink_dpipe_table_find()
>>
>Hi,
>
>Yes I had noticed this, but I was not sure if there is some other lock
>which is being used.
>
>If yes, then can you please tell me which lock is held in this case,
>and I can add that condition as well to list_for_each_entry_rcu() usage.
>
>And if no lock or rcu_read_lock is held then may be we should
>use rcu_read_lock/unlock here.
>
>Let me know what you think about this.
devlink->lock should be held since the beginning of
devlink_dpipe_table_register()
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 17:20 ` Jiri Pirko
@ 2020-02-21 17:35 ` Madhuparna Bhowmik
2020-02-21 17:54 ` Jiri Pirko
2020-02-23 11:03 ` Madhuparna Bhowmik
1 sibling, 1 reply; 14+ messages in thread
From: Madhuparna Bhowmik @ 2020-02-21 17:35 UTC (permalink / raw)
To: Jiri Pirko
Cc: madhuparnabhowmik10, jiri, davem, netdev, linux-kernel, joel,
frextrite, linux-kernel-mentees, paulmck
On Fri, Feb 21, 2020 at 06:20:08PM +0100, Jiri Pirko wrote:
> Fri, Feb 21, 2020 at 05:51:41PM 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
> >by default.
> >
> >Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
>
> Thanks.
>
> However, there is a callpath where not devlink lock neither rcu read is
> taken:
> devlink_dpipe_table_register()->devlink_dpipe_table_find()
>
Hi,
Yes I had noticed this, but I was not sure if there is some other lock
which is being used.
If yes, then can you please tell me which lock is held in this case,
and I can add that condition as well to list_for_each_entry_rcu() usage.
And if no lock or rcu_read_lock is held then may be we should
use rcu_read_lock/unlock here.
Let me know what you think about this.
Thank you,
Madhuparna
> I guess that was not the trace you were seeing, right?
>
>
> >---
> > net/core/devlink.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/net/core/devlink.c b/net/core/devlink.c
> >index 4c63c9a4c09e..3e8c94155d93 100644
> >--- a/net/core/devlink.c
> >+++ b/net/core/devlink.c
> >@@ -2107,7 +2107,8 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
> > {
> > 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;
> > }
> >--
> >2.17.1
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] net: core: devlink.c: Use built-in RCU list checking
2020-02-21 16:51 madhuparnabhowmik10
@ 2020-02-21 17:20 ` Jiri Pirko
2020-02-21 17:35 ` Madhuparna Bhowmik
2020-02-23 11:03 ` Madhuparna Bhowmik
2020-02-24 0:28 ` kbuild test robot
1 sibling, 2 replies; 14+ messages in thread
From: Jiri Pirko @ 2020-02-21 17:20 UTC (permalink / raw)
To: madhuparnabhowmik10
Cc: jiri, davem, netdev, linux-kernel, joel, frextrite,
linux-kernel-mentees, paulmck
Fri, Feb 21, 2020 at 05:51:41PM 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
>by default.
>
>Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Thanks.
However, there is a callpath where not devlink lock neither rcu read is
taken:
devlink_dpipe_table_register()->devlink_dpipe_table_find()
I guess that was not the trace you were seeing, right?
>---
> net/core/devlink.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 4c63c9a4c09e..3e8c94155d93 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -2107,7 +2107,8 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
> {
> 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;
> }
>--
>2.17.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] net: core: devlink.c: Use built-in RCU list checking
@ 2020-02-21 16:51 madhuparnabhowmik10
2020-02-21 17:20 ` Jiri Pirko
2020-02-24 0:28 ` kbuild test robot
0 siblings, 2 replies; 14+ messages in thread
From: madhuparnabhowmik10 @ 2020-02-21 16:51 UTC (permalink / raw)
To: jiri, davem
Cc: netdev, linux-kernel, joel, frextrite, linux-kernel-mentees,
paulmck, Madhuparna Bhowmik
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
by default.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
---
net/core/devlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4c63c9a4c09e..3e8c94155d93 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2107,7 +2107,8 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
{
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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-27 0:59 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 9:30 [PATCH] net: core: devlink.c: Use built-in RCU list checking madhuparnabhowmik10
2020-02-24 10:52 ` Jiri Pirko
2020-02-25 12:09 ` Madhuparna Bhowmik
-- strict thread matches above, loose matches on Subject: below --
2020-02-25 12:27 madhuparnabhowmik10
2020-02-25 13:06 ` Jiri Pirko
2020-02-27 0:59 ` David Miller
2020-02-21 16:51 madhuparnabhowmik10
2020-02-21 17:20 ` Jiri Pirko
2020-02-21 17:35 ` Madhuparna Bhowmik
2020-02-21 17:54 ` Jiri Pirko
2020-02-21 18:00 ` Madhuparna Bhowmik
2020-02-23 11:03 ` Madhuparna Bhowmik
2020-02-23 18:19 ` Jiri Pirko
2020-02-24 0:28 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).