All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipmi: Fix RCU list lockdep debugging warnings
@ 2020-02-28  8:17 ` Amol Grover
  0 siblings, 0 replies; 6+ messages in thread
From: Amol Grover @ 2020-02-28  8:17 UTC (permalink / raw)
  To: Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman
  Cc: openipmi-developer, linux-kernel, linux-kernel-mentees,
	John Garry, Joel Fernandes, Madhuparna Bhowmik,
	Paul E . Mckenney, Amol Grover

It is completely safe to traverse ipmi_interfaces and
intf->users under SRCU read lock using list_for_each_entry_rcu().
Tell lockdep about it as well else it will show false-positive
warnings as the one below.

Fixes the following false-positive warning and others that may follow.

[   29.772408] =============================
[   29.776863] WARNING: suspicious RCU usage
[   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
[   29.787046] -----------------------------
[   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
non-reader section!!

Reported-by: John Garry <john.garry@huawei.com>
Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Amol Grover <frextrite@gmail.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index cad9563f8f48..d202022c69de 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 	list_add(&watcher->link, &smi_watchers);
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		int intf_num = READ_ONCE(intf->intf_num);
 
 		if (intf_num == -1)
@@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
 		return -ENOMEM;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (intf->intf_num == if_num)
 			goto found;
 	}
@@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
 	struct ipmi_smi *intf;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (intf->intf_num == if_num)
 			goto found;
 	}
@@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	 * getting events.
 	 */
 	index = srcu_read_lock(&intf->users_srcu);
-	list_for_each_entry_rcu(user, &intf->users, link) {
+	list_for_each_entry_rcu(user, &intf->users, link,
+				srcu_read_lock_held(&intf->users_srcu)) {
 		if (!user->gets_events)
 			continue;
 
@@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
 		int index;
 
 		index = srcu_read_lock(&intf->users_srcu);
-		list_for_each_entry_rcu(user, &intf->users, link) {
+		list_for_each_entry_rcu(user, &intf->users, link,
+					srcu_read_lock_held(&intf->users_srcu)) {
 			if (user->handler->ipmi_watchdog_pretimeout)
 				user->handler->ipmi_watchdog_pretimeout(
 					user->handler_data);
@@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
 		return;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (atomic_read(&intf->event_waiters)) {
 			intf->ticks_to_req_ev--;
 			if (intf->ticks_to_req_ev == 0) {
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Linux-kernel-mentees] [PATCH] ipmi: Fix RCU list lockdep debugging warnings
@ 2020-02-28  8:17 ` Amol Grover
  0 siblings, 0 replies; 6+ messages in thread
From: Amol Grover @ 2020-02-28  8:17 UTC (permalink / raw)
  To: Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Paul E . Mckenney, John Garry, linux-kernel, Madhuparna Bhowmik,
	Joel Fernandes, openipmi-developer, linux-kernel-mentees

It is completely safe to traverse ipmi_interfaces and
intf->users under SRCU read lock using list_for_each_entry_rcu().
Tell lockdep about it as well else it will show false-positive
warnings as the one below.

Fixes the following false-positive warning and others that may follow.

[   29.772408] =============================
[   29.776863] WARNING: suspicious RCU usage
[   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
[   29.787046] -----------------------------
[   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
non-reader section!!

Reported-by: John Garry <john.garry@huawei.com>
Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Amol Grover <frextrite@gmail.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index cad9563f8f48..d202022c69de 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 	list_add(&watcher->link, &smi_watchers);
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		int intf_num = READ_ONCE(intf->intf_num);
 
 		if (intf_num == -1)
@@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
 		return -ENOMEM;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (intf->intf_num == if_num)
 			goto found;
 	}
@@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
 	struct ipmi_smi *intf;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (intf->intf_num == if_num)
 			goto found;
 	}
@@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
 	 * getting events.
 	 */
 	index = srcu_read_lock(&intf->users_srcu);
-	list_for_each_entry_rcu(user, &intf->users, link) {
+	list_for_each_entry_rcu(user, &intf->users, link,
+				srcu_read_lock_held(&intf->users_srcu)) {
 		if (!user->gets_events)
 			continue;
 
@@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
 		int index;
 
 		index = srcu_read_lock(&intf->users_srcu);
-		list_for_each_entry_rcu(user, &intf->users, link) {
+		list_for_each_entry_rcu(user, &intf->users, link,
+					srcu_read_lock_held(&intf->users_srcu)) {
 			if (user->handler->ipmi_watchdog_pretimeout)
 				user->handler->ipmi_watchdog_pretimeout(
 					user->handler_data);
@@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
 		return;
 
 	index = srcu_read_lock(&ipmi_interfaces_srcu);
-	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
+	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
+				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
 		if (atomic_read(&intf->event_waiters)) {
 			intf->ticks_to_req_ev--;
 			if (intf->ticks_to_req_ev == 0) {
-- 
2.25.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] ipmi: Fix RCU list lockdep debugging warnings
  2020-02-28  8:17 ` [Linux-kernel-mentees] " Amol Grover
@ 2020-02-28 11:49   ` John Garry
  -1 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2020-02-28 11:49 UTC (permalink / raw)
  To: Amol Grover, Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman
  Cc: openipmi-developer, linux-kernel, linux-kernel-mentees,
	Joel Fernandes, Madhuparna Bhowmik, Paul E . Mckenney

On 28/02/2020 08:17, Amol Grover wrote:
> It is completely safe to traverse ipmi_interfaces and
> intf->users under SRCU read lock using list_for_each_entry_rcu().
> Tell lockdep about it as well else it will show false-positive
> warnings as the one below.
> 
> Fixes the following false-positive warning and others that may follow.
> 
> [   29.772408] =============================
> [   29.776863] WARNING: suspicious RCU usage
> [   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
> [   29.787046] -----------------------------
> [   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
> non-reader section!!
> 
> Reported-by: John Garry <john.garry@huawei.com>
> Suggested-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Amol Grover <frextrite@gmail.com>

Tested-by: John Garry <john.garry@huawei.com>

Thanks, the warnings have gone away with this

> ---
>   drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index cad9563f8f48..d202022c69de 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
>   	list_add(&watcher->link, &smi_watchers);
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		int intf_num = READ_ONCE(intf->intf_num);
>   
>   		if (intf_num == -1)
> @@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
>   		return -ENOMEM;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (intf->intf_num == if_num)
>   			goto found;
>   	}
> @@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
>   	struct ipmi_smi *intf;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (intf->intf_num == if_num)
>   			goto found;
>   	}
> @@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
>   	 * getting events.
>   	 */
>   	index = srcu_read_lock(&intf->users_srcu);
> -	list_for_each_entry_rcu(user, &intf->users, link) {
> +	list_for_each_entry_rcu(user, &intf->users, link,
> +				srcu_read_lock_held(&intf->users_srcu)) {
>   		if (!user->gets_events)
>   			continue;
>   
> @@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
>   		int index;
>   
>   		index = srcu_read_lock(&intf->users_srcu);
> -		list_for_each_entry_rcu(user, &intf->users, link) {
> +		list_for_each_entry_rcu(user, &intf->users, link,
> +					srcu_read_lock_held(&intf->users_srcu)) {
>   			if (user->handler->ipmi_watchdog_pretimeout)
>   				user->handler->ipmi_watchdog_pretimeout(
>   					user->handler_data);
> @@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
>   		return;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (atomic_read(&intf->event_waiters)) {
>   			intf->ticks_to_req_ev--;
>   			if (intf->ticks_to_req_ev == 0) {
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] ipmi: Fix RCU list lockdep debugging warnings
@ 2020-02-28 11:49   ` John Garry
  0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2020-02-28 11:49 UTC (permalink / raw)
  To: Amol Grover, Corey Minyard, Arnd Bergmann, Greg Kroah-Hartman
  Cc: Paul E . Mckenney, linux-kernel, Madhuparna Bhowmik,
	Joel Fernandes, openipmi-developer, linux-kernel-mentees

On 28/02/2020 08:17, Amol Grover wrote:
> It is completely safe to traverse ipmi_interfaces and
> intf->users under SRCU read lock using list_for_each_entry_rcu().
> Tell lockdep about it as well else it will show false-positive
> warnings as the one below.
> 
> Fixes the following false-positive warning and others that may follow.
> 
> [   29.772408] =============================
> [   29.776863] WARNING: suspicious RCU usage
> [   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
> [   29.787046] -----------------------------
> [   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
> non-reader section!!
> 
> Reported-by: John Garry <john.garry@huawei.com>
> Suggested-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Amol Grover <frextrite@gmail.com>

Tested-by: John Garry <john.garry@huawei.com>

Thanks, the warnings have gone away with this

> ---
>   drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index cad9563f8f48..d202022c69de 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
>   	list_add(&watcher->link, &smi_watchers);
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		int intf_num = READ_ONCE(intf->intf_num);
>   
>   		if (intf_num == -1)
> @@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
>   		return -ENOMEM;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (intf->intf_num == if_num)
>   			goto found;
>   	}
> @@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
>   	struct ipmi_smi *intf;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (intf->intf_num == if_num)
>   			goto found;
>   	}
> @@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
>   	 * getting events.
>   	 */
>   	index = srcu_read_lock(&intf->users_srcu);
> -	list_for_each_entry_rcu(user, &intf->users, link) {
> +	list_for_each_entry_rcu(user, &intf->users, link,
> +				srcu_read_lock_held(&intf->users_srcu)) {
>   		if (!user->gets_events)
>   			continue;
>   
> @@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
>   		int index;
>   
>   		index = srcu_read_lock(&intf->users_srcu);
> -		list_for_each_entry_rcu(user, &intf->users, link) {
> +		list_for_each_entry_rcu(user, &intf->users, link,
> +					srcu_read_lock_held(&intf->users_srcu)) {
>   			if (user->handler->ipmi_watchdog_pretimeout)
>   				user->handler->ipmi_watchdog_pretimeout(
>   					user->handler_data);
> @@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
>   		return;
>   
>   	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>   		if (atomic_read(&intf->event_waiters)) {
>   			intf->ticks_to_req_ev--;
>   			if (intf->ticks_to_req_ev == 0) {
> 

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] ipmi: Fix RCU list lockdep debugging warnings
  2020-02-28  8:17 ` [Linux-kernel-mentees] " Amol Grover
@ 2020-02-28 17:44   ` Corey Minyard
  -1 siblings, 0 replies; 6+ messages in thread
From: Corey Minyard @ 2020-02-28 17:44 UTC (permalink / raw)
  To: Amol Grover
  Cc: Arnd Bergmann, Greg Kroah-Hartman, openipmi-developer,
	linux-kernel, linux-kernel-mentees, John Garry, Joel Fernandes,
	Madhuparna Bhowmik, Paul E . Mckenney

On Fri, Feb 28, 2020 at 01:47:33PM +0530, Amol Grover wrote:
> It is completely safe to traverse ipmi_interfaces and
> intf->users under SRCU read lock using list_for_each_entry_rcu().
> Tell lockdep about it as well else it will show false-positive
> warnings as the one below.

Thanks, this is queued for 5.7, and I hadded John Garry's test by.

-corey

> 
> Fixes the following false-positive warning and others that may follow.
> 
> [   29.772408] =============================
> [   29.776863] WARNING: suspicious RCU usage
> [   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
> [   29.787046] -----------------------------
> [   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
> non-reader section!!
> 
> Reported-by: John Garry <john.garry@huawei.com>
> Suggested-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Amol Grover <frextrite@gmail.com>
> ---
>  drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index cad9563f8f48..d202022c69de 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
>  	list_add(&watcher->link, &smi_watchers);
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		int intf_num = READ_ONCE(intf->intf_num);
>  
>  		if (intf_num == -1)
> @@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
>  		return -ENOMEM;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (intf->intf_num == if_num)
>  			goto found;
>  	}
> @@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
>  	struct ipmi_smi *intf;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (intf->intf_num == if_num)
>  			goto found;
>  	}
> @@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
>  	 * getting events.
>  	 */
>  	index = srcu_read_lock(&intf->users_srcu);
> -	list_for_each_entry_rcu(user, &intf->users, link) {
> +	list_for_each_entry_rcu(user, &intf->users, link,
> +				srcu_read_lock_held(&intf->users_srcu)) {
>  		if (!user->gets_events)
>  			continue;
>  
> @@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
>  		int index;
>  
>  		index = srcu_read_lock(&intf->users_srcu);
> -		list_for_each_entry_rcu(user, &intf->users, link) {
> +		list_for_each_entry_rcu(user, &intf->users, link,
> +					srcu_read_lock_held(&intf->users_srcu)) {
>  			if (user->handler->ipmi_watchdog_pretimeout)
>  				user->handler->ipmi_watchdog_pretimeout(
>  					user->handler_data);
> @@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
>  		return;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (atomic_read(&intf->event_waiters)) {
>  			intf->ticks_to_req_ev--;
>  			if (intf->ticks_to_req_ev == 0) {
> -- 
> 2.25.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] ipmi: Fix RCU list lockdep debugging warnings
@ 2020-02-28 17:44   ` Corey Minyard
  0 siblings, 0 replies; 6+ messages in thread
From: Corey Minyard @ 2020-02-28 17:44 UTC (permalink / raw)
  To: Amol Grover
  Cc: Paul E . Mckenney, Arnd Bergmann, John Garry, linux-kernel,
	Madhuparna Bhowmik, Joel Fernandes, openipmi-developer,
	linux-kernel-mentees

On Fri, Feb 28, 2020 at 01:47:33PM +0530, Amol Grover wrote:
> It is completely safe to traverse ipmi_interfaces and
> intf->users under SRCU read lock using list_for_each_entry_rcu().
> Tell lockdep about it as well else it will show false-positive
> warnings as the one below.

Thanks, this is queued for 5.7, and I hadded John Garry's test by.

-corey

> 
> Fixes the following false-positive warning and others that may follow.
> 
> [   29.772408] =============================
> [   29.776863] WARNING: suspicious RCU usage
> [   29.780915] 5.6.0-rc3-00001-g907305ae6618-dirty #1755 Not tainted
> [   29.787046] -----------------------------
> [   29.791100] drivers/char/ipmi/ipmi_msghandler.c:744 RCU-list traversed in
> non-reader section!!
> 
> Reported-by: John Garry <john.garry@huawei.com>
> Suggested-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Amol Grover <frextrite@gmail.com>
> ---
>  drivers/char/ipmi/ipmi_msghandler.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index cad9563f8f48..d202022c69de 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -741,7 +741,8 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
>  	list_add(&watcher->link, &smi_watchers);
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		int intf_num = READ_ONCE(intf->intf_num);
>  
>  		if (intf_num == -1)
> @@ -1188,7 +1189,8 @@ int ipmi_create_user(unsigned int          if_num,
>  		return -ENOMEM;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (intf->intf_num == if_num)
>  			goto found;
>  	}
> @@ -1241,7 +1243,8 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data)
>  	struct ipmi_smi *intf;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (intf->intf_num == if_num)
>  			goto found;
>  	}
> @@ -4098,7 +4101,8 @@ static int handle_read_event_rsp(struct ipmi_smi *intf,
>  	 * getting events.
>  	 */
>  	index = srcu_read_lock(&intf->users_srcu);
> -	list_for_each_entry_rcu(user, &intf->users, link) {
> +	list_for_each_entry_rcu(user, &intf->users, link,
> +				srcu_read_lock_held(&intf->users_srcu)) {
>  		if (!user->gets_events)
>  			continue;
>  
> @@ -4453,7 +4457,8 @@ static void handle_new_recv_msgs(struct ipmi_smi *intf)
>  		int index;
>  
>  		index = srcu_read_lock(&intf->users_srcu);
> -		list_for_each_entry_rcu(user, &intf->users, link) {
> +		list_for_each_entry_rcu(user, &intf->users, link,
> +					srcu_read_lock_held(&intf->users_srcu)) {
>  			if (user->handler->ipmi_watchdog_pretimeout)
>  				user->handler->ipmi_watchdog_pretimeout(
>  					user->handler_data);
> @@ -4746,7 +4751,8 @@ static void ipmi_timeout(struct timer_list *unused)
>  		return;
>  
>  	index = srcu_read_lock(&ipmi_interfaces_srcu);
> -	list_for_each_entry_rcu(intf, &ipmi_interfaces, link) {
> +	list_for_each_entry_rcu(intf, &ipmi_interfaces, link,
> +				srcu_read_lock_held(&ipmi_interfaces_srcu)) {
>  		if (atomic_read(&intf->event_waiters)) {
>  			intf->ticks_to_req_ev--;
>  			if (intf->ticks_to_req_ev == 0) {
> -- 
> 2.25.0
> 
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-28 17:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28  8:17 [PATCH] ipmi: Fix RCU list lockdep debugging warnings Amol Grover
2020-02-28  8:17 ` [Linux-kernel-mentees] " Amol Grover
2020-02-28 11:49 ` John Garry
2020-02-28 11:49   ` [Linux-kernel-mentees] " John Garry
2020-02-28 17:44 ` Corey Minyard
2020-02-28 17:44   ` [Linux-kernel-mentees] " Corey Minyard

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.