Re: [PATCH] ipv6: mcast: Add __must_hold() annotations.

[Kuniyuki Iwashima]

From: Brent Pappas <bpappas@xxxxxxxxxxxxxxx>
Date: Thu,  8 Aug 2024 15:02:55 -0400
> Add __must_hold(RCU) annotations to igmp6_mc_get_first(),
> igmp6_mc_get_next(), and igmp6_mc_get_idx() to signify that they are
> meant to be called in RCU critical sections.
> 
> Signed-off-by: Brent Pappas <bpappas@xxxxxxxxxxxxxxx>
> ---
>  net/ipv6/mcast.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 7ba01d8cfbae..843d0d065242 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -22,6 +22,7 @@
>   *		- MLDv2 support
>   */
>  
> +#include "linux/compiler_types.h"

Why "" ?

Btw, I think for_each_netdev_rcu() / rcu_dereference() in touched
functions are enough to cleary annotate RCU is needed there.

Even without it, I prefer rcu_read_lock_held(), I'm not sure to
what extent sparse can analyse functions statically though.


>  #include <linux/module.h>
>  #include <linux/errno.h>
>  #include <linux/types.h>
> @@ -2861,6 +2862,7 @@ struct igmp6_mc_iter_state {
>  #define igmp6_mc_seq_private(seq)	((struct igmp6_mc_iter_state *)(seq)->private)
>  
>  static inline struct ifmcaddr6 *igmp6_mc_get_first(struct seq_file *seq)
> +	__must_hold(RCU)
>  {
>  	struct ifmcaddr6 *im = NULL;
>  	struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
> @@ -2882,7 +2884,9 @@ static inline struct ifmcaddr6 *igmp6_mc_get_first(struct seq_file *seq)
>  	return im;
>  }
>  
> -static struct ifmcaddr6 *igmp6_mc_get_next(struct seq_file *seq, struct ifmcaddr6 *im)
> +static struct ifmcaddr6 *igmp6_mc_get_next(struct seq_file *seq,
> +					   struct ifmcaddr6 *im)
> +	__must_hold(RCU)
>  {
>  	struct igmp6_mc_iter_state *state = igmp6_mc_seq_private(seq);
>  
> @@ -2902,6 +2906,7 @@ static struct ifmcaddr6 *igmp6_mc_get_next(struct seq_file *seq, struct ifmcaddr
>  }
>  
>  static struct ifmcaddr6 *igmp6_mc_get_idx(struct seq_file *seq, loff_t pos)
> +	__must_hold(RCU)
>  {
>  	struct ifmcaddr6 *im = igmp6_mc_get_first(seq);
>  	if (im)
> -- 
> 2.46.0