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

Jesse Gross jesse at nicira.com
Tue Jul 1 01:01:09 UTC 2014


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




More information about the dev mailing list