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

From: Kuniyuki Iwashima
Date: Thu Aug 08 2024 - 16:24:27 EST


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