LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH net-next 2/6] ipvs: fix sparse warnings for ip_vs_conn listin

To: Julian Anastasov <ja@xxxxxx>
Subject: Re: [PATCH net-next 2/6] ipvs: fix sparse warnings for ip_vs_conn listing
Cc: lvs-devel@xxxxxxxxxxxxxxx, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Thu, 18 Apr 2013 20:46:58 +0900
On Wed, Apr 17, 2013 at 11:50:46PM +0300, Julian Anastasov wrote:
>       kbuild test robot reports for sparse warnings
> in commit 088339a57d6042 ("ipvs: convert connection locking"):
> 
> net/netfilter/ipvs/ip_vs_conn.c:962:13: warning: context imbalance
>       in 'ip_vs_conn_array' - wrong count at exit
> include/linux/rcupdate.h:326:30: warning: context imbalance in
>       'ip_vs_conn_seq_next' - unexpected unlock
> include/linux/rcupdate.h:326:30: warning: context imbalance in
>       'ip_vs_conn_seq_stop' - unexpected unlock
> 
>       Fix it by running ip_vs_conn_array under RCU lock
> to avoid conditional locking and by adding proper RCU
> annotations.

Thanks, this seems correct to me.
I have queued it up in ipvs-next.

> 
> Signed-off-by: Julian Anastasov <ja@xxxxxx>
> ---
>  net/netfilter/ipvs/ip_vs_conn.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index de64758..a083bda 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -966,7 +966,6 @@ static void *ip_vs_conn_array(struct seq_file *seq, 
> loff_t pos)
>       struct ip_vs_iter_state *iter = seq->private;
>  
>       for (idx = 0; idx < ip_vs_conn_tab_size; idx++) {
> -             rcu_read_lock();
>               hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
>                       /* __ip_vs_conn_get() is not needed by
>                        * ip_vs_conn_seq_show and ip_vs_conn_sync_seq_show
> @@ -977,16 +976,19 @@ static void *ip_vs_conn_array(struct seq_file *seq, 
> loff_t pos)
>                       }
>               }
>               rcu_read_unlock();
> +             rcu_read_lock();
>       }
>  
>       return NULL;
>  }
>  
>  static void *ip_vs_conn_seq_start(struct seq_file *seq, loff_t *pos)
> +     __acquires(RCU)
>  {
>       struct ip_vs_iter_state *iter = seq->private;
>  
>       iter->l = NULL;
> +     rcu_read_lock();
>       return *pos ? ip_vs_conn_array(seq, *pos - 1) :SEQ_START_TOKEN;
>  }
>  
> @@ -1006,28 +1008,24 @@ static void *ip_vs_conn_seq_next(struct seq_file 
> *seq, void *v, loff_t *pos)
>       e = rcu_dereference(hlist_next_rcu(&cp->c_list));
>       if (e)
>               return hlist_entry(e, struct ip_vs_conn, c_list);
> -     rcu_read_unlock();
>  
>       idx = l - ip_vs_conn_tab;
>       while (++idx < ip_vs_conn_tab_size) {
> -             rcu_read_lock();
>               hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
>                       iter->l = &ip_vs_conn_tab[idx];
>                       return cp;
>               }
>               rcu_read_unlock();
> +             rcu_read_lock();
>       }
>       iter->l = NULL;
>       return NULL;
>  }
>  
>  static void ip_vs_conn_seq_stop(struct seq_file *seq, void *v)
> +     __releases(RCU)
>  {
> -     struct ip_vs_iter_state *iter = seq->private;
> -     struct hlist_head *l = iter->l;
> -
> -     if (l)
> -             rcu_read_unlock();
> +     rcu_read_unlock();
>  }
>  
>  static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
> -- 
> 1.7.3.4
> 
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>