[ovs-dev] [PATCH] datapath: Change u64_stats_* to use _irq instead of _bh().

Pritesh Kothari (pritkoth) pritkoth at cisco.com
Tue Jul 1 18:06:21 UTC 2014


Acked-by: Pritesh Kothari <pritesh.kothari at cisco.com>

On Jun 30, 2014, at 6:01 PM, Jesse Gross <jesse at nicira.com> wrote:

> The upstream u64_stats API has been changed to remove the _bh()
> versions and switch all consumers to use IRQ safe variants instead.
> This was done to be safe for netpoll generated packets, which can
> occur in hard IRQ context. From a safety perspective, this doesn't
> directly affect OVS since it doesn't support netpoll. However, this
> change has been backported to older kernels so OVS needs to use the
> new API to compile.
> 
> Signed-off-by: Jesse Gross <jesse at nicira.com>
> ---
> acinclude.m4                                       |  2 ++
> datapath/datapath.c                                |  4 +--
> .../linux/compat/include/linux/u64_stats_sync.h    | 37 ++++++++++------------
> datapath/vport.c                                   |  4 +--
> 4 files changed, 23 insertions(+), 24 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index c761366..aa9ffcd 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -348,6 +348,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
> 
>   OVS_GREP_IFELSE([$KSRC/include/linux/percpu.h], [this_cpu_ptr])
> 
> +  OVS_GREP_IFELSE([$KSRC/include/linux/u64_stats_sync.h], [u64_stats_fetch_begin_irq])
> +
>   OVS_GREP_IFELSE([$KSRC/include/linux/openvswitch.h], [openvswitch_handle_frame_hook],
>                   [OVS_DEFINE([HAVE_RHEL_OVS_HOOK])])
> 
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index e504fee..a7a6c3c 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -673,9 +673,9 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats,
> 		percpu_stats = per_cpu_ptr(dp->stats_percpu, i);
> 
> 		do {
> -			start = u64_stats_fetch_begin_bh(&percpu_stats->sync);
> +			start = u64_stats_fetch_begin_irq(&percpu_stats->sync);
> 			local_stats = *percpu_stats;
> -		} while (u64_stats_fetch_retry_bh(&percpu_stats->sync, start));
> +		} while (u64_stats_fetch_retry_irq(&percpu_stats->sync, start));
> 
> 		stats->n_hit += local_stats.n_hit;
> 		stats->n_missed += local_stats.n_missed;
> diff --git a/datapath/linux/compat/include/linux/u64_stats_sync.h b/datapath/linux/compat/include/linux/u64_stats_sync.h
> index 234fd91..9342f73 100644
> --- a/datapath/linux/compat/include/linux/u64_stats_sync.h
> +++ b/datapath/linux/compat/include/linux/u64_stats_sync.h
> @@ -3,7 +3,8 @@
> 
> #include <linux/version.h>
> 
> -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36)
> +#if defined(HAVE_U64_STATS_FETCH_BEGIN_IRQ) && \
> +    LINUX_VERSION_CODE >= KERNEL_VERSION(3,13,0)
> #include_next <linux/u64_stats_sync.h>
> #else
> 
> @@ -33,8 +34,8 @@
>  *    (On UP, there is no seqcount_t protection, a reader allowing interrupts could
>  *     read partial values)
>  *
> - * 7) For softirq uses, readers can use u64_stats_fetch_begin_bh() and
> - *    u64_stats_fetch_retry_bh() helpers
> + * 7) For irq or softirq uses, readers can use u64_stats_fetch_begin_irq() and
> + *    u64_stats_fetch_retry_irq() helpers
>  *
>  * Usage :
>  *
> @@ -73,6 +74,12 @@ struct u64_stats_sync {
> #endif
> };
> 
> +#if BITS_PER_LONG == 32 && defined(CONFIG_SMP)
> +# define u64_stats_init(syncp)  seqcount_init(syncp.seq)
> +#else
> +# define u64_stats_init(syncp)  do { } while (0)
> +#endif
> +
> static inline void u64_stats_update_begin(struct u64_stats_sync *syncp)
> {
> #if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> @@ -113,46 +120,36 @@ static inline bool u64_stats_fetch_retry(const struct u64_stats_sync *syncp,
> }
> 
> /*
> - * In case softirq handlers can update u64 counters, readers can use following helpers
> + * In case irq handlers can update u64 counters, readers can use following helpers
>  * - SMP 32bit arches use seqcount protection, irq safe.
> - * - UP 32bit must disable BH.
> + * - UP 32bit must disable irqs.
>  * - 64bit have no problem atomically reading u64 values, irq safe.
>  */
> -static inline unsigned int u64_stats_fetch_begin_bh(const struct u64_stats_sync *syncp)
> +static inline unsigned int u64_stats_fetch_begin_irq(const struct u64_stats_sync *syncp)
> {
> #if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> 	return read_seqcount_begin(&syncp->seq);
> #else
> #if BITS_PER_LONG==32
> -	local_bh_disable();
> +	local_irq_disable();
> #endif
> 	return 0;
> #endif
> }
> 
> -static inline bool u64_stats_fetch_retry_bh(const struct u64_stats_sync *syncp,
> +static inline bool u64_stats_fetch_retry_irq(const struct u64_stats_sync *syncp,
> 					 unsigned int start)
> {
> #if BITS_PER_LONG==32 && defined(CONFIG_SMP)
> 	return read_seqcount_retry(&syncp->seq, start);
> #else
> #if BITS_PER_LONG==32
> -	local_bh_enable();
> +	local_irq_enable();
> #endif
> 	return false;
> #endif
> }
> 
> -#endif /* Linux kernel < 2.6.36 */
> -
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> -
> -#if BITS_PER_LONG == 32 && defined(CONFIG_SMP)
> -# define u64_stats_init(syncp)  seqcount_init(syncp.seq)
> -#else
> -# define u64_stats_init(syncp)  do { } while (0)
> -#endif
> -
> -#endif
> +#endif /* !HAVE_U64_STATS_FETCH_BEGIN_IRQ || kernel < 3.13 */
> 
> #endif /* _LINUX_U64_STATS_SYNC_WRAPPER_H */
> diff --git a/datapath/vport.c b/datapath/vport.c
> index 897f221..42ee726 100644
> --- a/datapath/vport.c
> +++ b/datapath/vport.c
> @@ -311,9 +311,9 @@ void ovs_vport_get_stats(struct vport *vport, struct ovs_vport_stats *stats)
> 		percpu_stats = per_cpu_ptr(vport->percpu_stats, i);
> 
> 		do {
> -			start = u64_stats_fetch_begin_bh(&percpu_stats->syncp);
> +			start = u64_stats_fetch_begin_irq(&percpu_stats->syncp);
> 			local_stats = *percpu_stats;
> -		} while (u64_stats_fetch_retry_bh(&percpu_stats->syncp, start));
> +		} while (u64_stats_fetch_retry_irq(&percpu_stats->syncp, start));
> 
> 		stats->rx_bytes		+= local_stats.rx_bytes;
> 		stats->rx_packets	+= local_stats.rx_packets;
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list