* [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 17:18 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141117171005.22333.96544.stgit@ahduyck-server>
There are a number of situations where the mandatory barriers rmb() and
wmb() are used to order memory/memory operations in the device drivers
and those barriers are much heavier than they actually need to be. For
example in the case of PowerPC wmb() calls the heavy-weight sync
instruction when for memory/memory operations all that is really needed is
an lsync or eieio instruction.
This commit adds a fast (and loose) version of the mandatory memory
barriers rmb() and wmb(). The prefix to the name is actually based on the
version of the functions that already exist in the mips and tile trees.
However I thought it applicable since it gets at what we are trying to
accomplish with these barriers and somethat implies their risky nature.
These new barriers are not as safe as the standard rmb() and wmb().
Specifically they do not guarantee ordering between cache-enabled and
cache-inhibited memories. The primary use case for these would be to
enforce ordering of memory reads/writes when accessing cache-enabled memory
that is shared between the CPU and a device.
It may also be noted that there is no fast_mb(). This is due to the fact
that most architectures didn't seem to have a good way to do a full memory
barrier quickly and so they usually resorted to an mb() for their smp_mb
call. As such there no point in adding a fast_mb() function if it is going
to map to mb() for all architectures anyway.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
Documentation/memory-barriers.txt | 41 +++++++++++++++++++++++++++++++++++
arch/arm/include/asm/barrier.h | 4 +++
arch/arm64/include/asm/barrier.h | 3 +++
arch/ia64/include/asm/barrier.h | 3 +++
arch/metag/include/asm/barrier.h | 14 ++++++------
arch/powerpc/include/asm/barrier.h | 22 +++++++++++--------
arch/s390/include/asm/barrier.h | 2 ++
arch/sparc/include/asm/barrier_64.h | 3 +++
arch/x86/include/asm/barrier.h | 11 ++++++---
arch/x86/um/asm/barrier.h | 13 ++++++-----
include/asm-generic/barrier.h | 8 +++++++
11 files changed, 98 insertions(+), 26 deletions(-)
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 22a969c..fe55dea 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
operations" subsection for information on where to use these.
+ (*) fast_wmb();
+ (*) fast_rmb();
+
+ These are for use with memory based device I/O to guarantee the ordering
+ of cache-enabled writes or reads with respect to other writes or reads
+ to cache-enabled memory.
+
+ For example, consider a device driver that shares memory with a device
+ and uses a descriptor status value to indicate if the descriptor belongs
+ to the device or the CPU, and a doorbell to notify it when new
+ descriptors are available:
+
+ if (desc->status != DEVICE_OWN) {
+ /* do not read data until we own descriptor */
+ fast_rmb();
+
+ /* read/modify data */
+ read_data = desc->data;
+ desc->data = write_data;
+
+ /* flush modifications before status update */
+ fast_wmb();
+
+ /* assign ownership */
+ desc->status = DEVICE_OWN;
+
+ /* force memory to sync before notifying device via MMIO */
+ wmb();
+
+ /* notify device of new descriptors */
+ writel(DESC_NOTIFY, doorbell);
+ }
+
+ The fast_rmb() allows us guarantee the device has released ownership
+ before we read the data from the descriptor, and he fast_wmb() allows
+ us to guarantee the data is written to the descriptor before the device
+ can see it now has ownership. The wmb() is needed to guarantee that the
+ cache-enabled memory writes have completed before attempting a write to
+ the cache-inhibited MMIO region.
+
+
MMIO WRITE BARRIER
------------------
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
index c6a3e73..c57903c 100644
--- a/arch/arm/include/asm/barrier.h
+++ b/arch/arm/include/asm/barrier.h
@@ -43,10 +43,14 @@
#define mb() do { dsb(); outer_sync(); } while (0)
#define rmb() dsb()
#define wmb() do { dsb(st); outer_sync(); } while (0)
+#define fast_rmb() dmb()
+#define fast_wmb() dmb(st)
#else
#define mb() barrier()
#define rmb() barrier()
#define wmb() barrier()
+#define fast_rmb() barrier()
+#define fast_wmb() barrier()
#endif
#ifndef CONFIG_SMP
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 6389d60..3ca1a15 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -32,6 +32,9 @@
#define rmb() dsb(ld)
#define wmb() dsb(st)
+#define fast_rmb() dmb(ld)
+#define fast_wmb() dmb(st)
+
#ifndef CONFIG_SMP
#define smp_mb() barrier()
#define smp_rmb() barrier()
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index e8fffb0..997f5b0 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -39,6 +39,9 @@
#define rmb() mb()
#define wmb() mb()
+#define fast_rmb() mb()
+#define fast_wmb() mb()
+
#ifdef CONFIG_SMP
# define smp_mb() mb()
#else
diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
index 6d8b8c9..edddb6d 100644
--- a/arch/metag/include/asm/barrier.h
+++ b/arch/metag/include/asm/barrier.h
@@ -4,8 +4,6 @@
#include <asm/metag_mem.h>
#define nop() asm volatile ("NOP")
-#define mb() wmb()
-#define rmb() barrier()
#ifdef CONFIG_METAG_META21
@@ -41,11 +39,13 @@ static inline void wr_fence(void)
#endif /* !CONFIG_METAG_META21 */
-static inline void wmb(void)
-{
- /* flush writes through the write combiner */
- wr_fence();
-}
+/* flush writes through the write combiner */
+#define mb() wr_fence()
+#define rmb() barrier()
+#define wmb() mb()
+
+#define fast_rmb() rmb()
+#define fast_wmb() wmb()
#ifndef CONFIG_SMP
#define fence() do { } while (0)
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index cb6d66c..f480097 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -36,22 +36,20 @@
#define set_mb(var, value) do { var = value; mb(); } while (0)
-#ifdef CONFIG_SMP
-
#ifdef __SUBARCH_HAS_LWSYNC
# define SMPWMB LWSYNC
#else
# define SMPWMB eieio
#endif
-#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
+#define fast_rmb() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
+#define fast_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
+#ifdef CONFIG_SMP
#define smp_mb() mb()
-#define smp_rmb() __lwsync()
-#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
+#define smp_rmb() fast_rmb()
+#define smp_wmb() fast_wmb()
#else
-#define __lwsync() barrier()
-
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
@@ -69,10 +67,16 @@
#define data_barrier(x) \
asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
+/*
+ * The use of smp_rmb() in these functions are actually meant to map from
+ * smp_rmb()->fast_rmb()->LWSYNC. This way if smp is disabled then
+ * smp_rmb()->barrier(), or if the platform doesn't support lwsync it will
+ * map to the more heavy-weight sync.
+ */
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_rmb(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
@@ -80,7 +84,7 @@ do { \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_rmb(); \
___p1; \
})
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index 33d191d..9a31301 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -24,6 +24,8 @@
#define rmb() mb()
#define wmb() mb()
+#define fast_rmb() rmb()
+#define fast_wmb() wmb()
#define smp_mb() mb()
#define smp_rmb() rmb()
#define smp_wmb() wmb()
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 6c974c0..eac2777 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -37,6 +37,9 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
+#define fast_rmb() rmb()
+#define fast_wmb() wmb()
+
#define set_mb(__var, __value) \
do { __var = __value; membar_safe("#StoreLoad"); } while(0)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index 5238000..cf440e7 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -24,13 +24,16 @@
#define wmb() asm volatile("sfence" ::: "memory")
#endif
-#ifdef CONFIG_SMP
-#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
-# define smp_rmb() rmb()
+#define fast_rmb() rmb()
#else
-# define smp_rmb() barrier()
+#define fast_rmb() barrier()
#endif
+#define fast_wmb() barrier()
+
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() fast_rmb()
#define smp_wmb() barrier()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* !SMP */
diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
index d6511d9..2c0405d 100644
--- a/arch/x86/um/asm/barrier.h
+++ b/arch/x86/um/asm/barrier.h
@@ -29,17 +29,18 @@
#endif /* CONFIG_X86_32 */
-#ifdef CONFIG_SMP
-
-#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
-#define smp_rmb() rmb()
+#define fast_rmb() rmb()
#else /* CONFIG_X86_PPRO_FENCE */
-#define smp_rmb() barrier()
+#define fast_rmb() barrier()
#endif /* CONFIG_X86_PPRO_FENCE */
+#define fast_wmb() barrier()
-#define smp_wmb() barrier()
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() fast_rmb()
+#define smp_wmb() fast_wmb()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* CONFIG_SMP */
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 1402fa8..c1d60b9 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -42,6 +42,14 @@
#define wmb() mb()
#endif
+#ifndef fast_rmb
+#define fast_rmb() rmb()
+#endif
+
+#ifndef fast_wmb
+#define fast_wmb() wmb()
+#endif
+
#ifndef read_barrier_depends
#define read_barrier_depends() do { } while (0)
#endif
^ permalink raw reply related
* [PATCH 3/4] r8169: Use fast_rmb() and fast_wmb() for DescOwn checks
From: Alexander Duyck @ 2014-11-17 17:18 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141117171005.22333.96544.stgit@ahduyck-server>
The r8169 use a pair of wmb() calls when setting up the descriptor rings.
The first is to synchronize the descriptor data with the descriptor status,
and the second is to synchronize the descriptor status with the use of the
MMIO doorbell to notify the device that descriptors are ready. This can come
at a heavy price on some systems, and is not really necessary on systems such
as x86 as a simple barrier() would suffice to order store/store accesses.
As such we can replace the first memory barrier with fast_wmb() to reduce
the cost for these accesses.
In addition the r8169 uses a rmb() to prevent compiler optimization in the
cleanup paths, however by moving the barrier down a few lines and
replacing with with a fast_rmb() we should be able to use it to guarantee
descriptor accesses do not occur until the device has updated the DescOwn
bit from its end.
One last change I made is to move the update of cur_tx in the xmit path to
after the wmb. This way we can guarantee the device and all CPUs should
see the DescOwn update before they see the cur_tx value update.
Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/realtek/r8169.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index cf154f7..be79447 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6601,6 +6601,9 @@ static inline void rtl8169_mark_to_asic(struct RxDesc *desc, u32 rx_buf_sz)
{
u32 eor = le32_to_cpu(desc->opts1) & RingEnd;
+ /* Force memory writes to complete before releasing descriptor */
+ fast_wmb();
+
desc->opts1 = cpu_to_le32(DescOwn | eor | rx_buf_sz);
}
@@ -6608,7 +6611,6 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
u32 rx_buf_sz)
{
desc->addr = cpu_to_le64(mapping);
- wmb();
rtl8169_mark_to_asic(desc, rx_buf_sz);
}
@@ -7077,16 +7079,18 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
skb_tx_timestamp(skb);
- wmb();
+ /* Force memory writes to complete before releasing descriptor */
+ fast_wmb();
/* Anti gcc 2.95.3 bugware (sic) */
status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
txd->opts1 = cpu_to_le32(status);
- tp->cur_tx += frags + 1;
-
+ /* Force all memory writes to complete before notifying device */
wmb();
+ tp->cur_tx += frags + 1;
+
RTL_W8(TxPoll, NPQ);
mmiowb();
@@ -7185,11 +7189,16 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
struct ring_info *tx_skb = tp->tx_skb + entry;
u32 status;
- rmb();
status = le32_to_cpu(tp->TxDescArray[entry].opts1);
if (status & DescOwn)
break;
+ /* This barrier is needed to keep us from reading
+ * any other fields out of the Tx descriptor until
+ * we know the status of DescOwn
+ */
+ fast_rmb();
+
rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
tp->TxDescArray + entry);
if (status & LastFrag) {
@@ -7284,11 +7293,16 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
struct RxDesc *desc = tp->RxDescArray + entry;
u32 status;
- rmb();
status = le32_to_cpu(desc->opts1) & tp->opts1_mask;
-
if (status & DescOwn)
break;
+
+ /* This barrier is needed to keep us from reading
+ * any other fields out of the Rx descriptor until
+ * we know the status of DescOwn
+ */
+ fast_rmb();
+
if (unlikely(status & RxRES)) {
netif_info(tp, rx_err, dev, "Rx ERROR. status = %08x\n",
status);
@@ -7350,7 +7364,6 @@ process_pkt:
}
release_descriptor:
desc->opts2 = 0;
- wmb();
rtl8169_mark_to_asic(desc, rx_buf_sz);
}
^ permalink raw reply related
* [PATCH 4/4] fm10k/igb/ixgbe: Use fast_rmb on Rx descriptor reads
From: Alexander Duyck @ 2014-11-17 17:18 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141117171005.22333.96544.stgit@ahduyck-server>
This change makes it so that fast_rmb is used when reading the Rx
descriptor. The advantage of fast_rmb is that it allows for a much
lower cost barrier on x86, powerpc, arm, and arm64 architectures than a
traditional memory barrier when dealing with reads that only have to
synchronize to system memory.
In addition I have updated the code so that it just checks to see if any
bits have been set instead of just the DD bit since the DD bit will always
be set as a part of a descriptor write-back so we just need to check for a
non-zero value being present at that memory location rather than just
checking for any specific bit. This allows the code itself to appear much
cleaner and allows the compiler more room to optimize.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Matthew Vick <matthew.vick@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 +++---
drivers/net/ethernet/intel/igb/igb_main.c | 6 +++---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++-----
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index e645af4..ba959b9 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -620,14 +620,14 @@ static bool fm10k_clean_rx_irq(struct fm10k_q_vector *q_vector,
rx_desc = FM10K_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!fm10k_test_staterr(rx_desc, FM10K_RXD_STATUS_DD))
+ if (!rx_desc->d.staterr)
break;
/* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STATUS_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ fast_rmb();
/* retrieve a buffer from the ring */
skb = fm10k_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a2d72a8..3dbbaa8 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6918,14 +6918,14 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
rx_desc = IGB_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_DD))
+ if (!rx_desc->wb.upper.status_error)
break;
/* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STAT_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ fast_rmb();
/* retrieve a buffer from the ring */
skb = igb_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..890d740 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2003,15 +2003,14 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
rx_desc = IXGBE_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
+ if (!rx_desc->wb.upper.status_error)
break;
- /*
- * This memory barrier is needed to keep us from reading
+ /* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STAT_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ fast_rmb();
/* retrieve a buffer from the ring */
skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);
^ permalink raw reply related
* Re: [PATCHv2 net 0/4] Implement ndo_gso_check() for vxlan nics
From: Joe Stringer @ 2014-11-17 17:50 UTC (permalink / raw)
To: David Miller
Cc: netdev, sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev,
therbert, gerlitz.or, linux-kernel
In-Reply-To: <20141114.171321.696698672872443302.davem@davemloft.net>
On Friday, November 14, 2014 14:13:21 David Miller wrote:
> From: Joe Stringer <joestringer@nicira.com>
> Date: Thu, 13 Nov 2014 16:38:11 -0800
>
> > Most NICs that report NETIF_F_GSO_UDP_TUNNEL support VXLAN, and not other
> > UDP-based encapsulation protocols where the format and size of the header
> > may differ. This patch series implements a generic ndo_gso_check() for
> > detecting VXLAN, then reuses it for these NICs.
> >
> > Implementation shamelessly stolen from Tom Herbert (with minor fixups):
> > http://thread.gmane.org/gmane.linux.network/332428/focus=333111
> >
> > v2: Drop i40e/fm10k patches (code diverged; handling separately).
> >
> > Refactor common code into vxlan_gso_check() helper.
> > Minor style fixes.
>
> Series applied, thanks Joe.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Benjamin Herrenschmidt @ 2014-11-17 20:04 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz,
heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
matthew.vick, geert, jeffrey.t.kirsher, romieu, paulmck, nic_swsd,
will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
fweisbec, davem
In-Reply-To: <20141117171812.22333.90395.stgit@ahduyck-server>
On Mon, 2014-11-17 at 09:18 -0800, Alexander Duyck wrote:
> There are a number of situations where the mandatory barriers rmb() and
> wmb() are used to order memory/memory operations in the device drivers
> and those barriers are much heavier than they actually need to be. For
> example in the case of PowerPC wmb() calls the heavy-weight sync
> instruction when for memory/memory operations all that is really needed is
> an lsync or eieio instruction.
So essentially those are the same as the smp_* variants but not nop'ed
out on !CONFIG_SMP right ?
Ben.
> This commit adds a fast (and loose) version of the mandatory memory
> barriers rmb() and wmb(). The prefix to the name is actually based on the
> version of the functions that already exist in the mips and tile trees.
> However I thought it applicable since it gets at what we are trying to
> accomplish with these barriers and somethat implies their risky nature.
>
> These new barriers are not as safe as the standard rmb() and wmb().
> Specifically they do not guarantee ordering between cache-enabled and
> cache-inhibited memories. The primary use case for these would be to
> enforce ordering of memory reads/writes when accessing cache-enabled memory
> that is shared between the CPU and a device.
>
> It may also be noted that there is no fast_mb(). This is due to the fact
> that most architectures didn't seem to have a good way to do a full memory
> barrier quickly and so they usually resorted to an mb() for their smp_mb
> call. As such there no point in adding a fast_mb() function if it is going
> to map to mb() for all architectures anyway.
>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Cc: Michael Ellerman <michael@ellerman.id.au>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
> Documentation/memory-barriers.txt | 41 +++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/barrier.h | 4 +++
> arch/arm64/include/asm/barrier.h | 3 +++
> arch/ia64/include/asm/barrier.h | 3 +++
> arch/metag/include/asm/barrier.h | 14 ++++++------
> arch/powerpc/include/asm/barrier.h | 22 +++++++++++--------
> arch/s390/include/asm/barrier.h | 2 ++
> arch/sparc/include/asm/barrier_64.h | 3 +++
> arch/x86/include/asm/barrier.h | 11 ++++++---
> arch/x86/um/asm/barrier.h | 13 ++++++-----
> include/asm-generic/barrier.h | 8 +++++++
> 11 files changed, 98 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 22a969c..fe55dea 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
> operations" subsection for information on where to use these.
>
>
> + (*) fast_wmb();
> + (*) fast_rmb();
> +
> + These are for use with memory based device I/O to guarantee the ordering
> + of cache-enabled writes or reads with respect to other writes or reads
> + to cache-enabled memory.
> +
> + For example, consider a device driver that shares memory with a device
> + and uses a descriptor status value to indicate if the descriptor belongs
> + to the device or the CPU, and a doorbell to notify it when new
> + descriptors are available:
> +
> + if (desc->status != DEVICE_OWN) {
> + /* do not read data until we own descriptor */
> + fast_rmb();
> +
> + /* read/modify data */
> + read_data = desc->data;
> + desc->data = write_data;
> +
> + /* flush modifications before status update */
> + fast_wmb();
> +
> + /* assign ownership */
> + desc->status = DEVICE_OWN;
> +
> + /* force memory to sync before notifying device via MMIO */
> + wmb();
> +
> + /* notify device of new descriptors */
> + writel(DESC_NOTIFY, doorbell);
> + }
> +
> + The fast_rmb() allows us guarantee the device has released ownership
> + before we read the data from the descriptor, and he fast_wmb() allows
> + us to guarantee the data is written to the descriptor before the device
> + can see it now has ownership. The wmb() is needed to guarantee that the
> + cache-enabled memory writes have completed before attempting a write to
> + the cache-inhibited MMIO region.
> +
> +
> MMIO WRITE BARRIER
> ------------------
>
> diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
> index c6a3e73..c57903c 100644
> --- a/arch/arm/include/asm/barrier.h
> +++ b/arch/arm/include/asm/barrier.h
> @@ -43,10 +43,14 @@
> #define mb() do { dsb(); outer_sync(); } while (0)
> #define rmb() dsb()
> #define wmb() do { dsb(st); outer_sync(); } while (0)
> +#define fast_rmb() dmb()
> +#define fast_wmb() dmb(st)
> #else
> #define mb() barrier()
> #define rmb() barrier()
> #define wmb() barrier()
> +#define fast_rmb() barrier()
> +#define fast_wmb() barrier()
> #endif
>
> #ifndef CONFIG_SMP
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 6389d60..3ca1a15 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -32,6 +32,9 @@
> #define rmb() dsb(ld)
> #define wmb() dsb(st)
>
> +#define fast_rmb() dmb(ld)
> +#define fast_wmb() dmb(st)
> +
> #ifndef CONFIG_SMP
> #define smp_mb() barrier()
> #define smp_rmb() barrier()
> diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
> index e8fffb0..997f5b0 100644
> --- a/arch/ia64/include/asm/barrier.h
> +++ b/arch/ia64/include/asm/barrier.h
> @@ -39,6 +39,9 @@
> #define rmb() mb()
> #define wmb() mb()
>
> +#define fast_rmb() mb()
> +#define fast_wmb() mb()
> +
> #ifdef CONFIG_SMP
> # define smp_mb() mb()
> #else
> diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
> index 6d8b8c9..edddb6d 100644
> --- a/arch/metag/include/asm/barrier.h
> +++ b/arch/metag/include/asm/barrier.h
> @@ -4,8 +4,6 @@
> #include <asm/metag_mem.h>
>
> #define nop() asm volatile ("NOP")
> -#define mb() wmb()
> -#define rmb() barrier()
>
> #ifdef CONFIG_METAG_META21
>
> @@ -41,11 +39,13 @@ static inline void wr_fence(void)
>
> #endif /* !CONFIG_METAG_META21 */
>
> -static inline void wmb(void)
> -{
> - /* flush writes through the write combiner */
> - wr_fence();
> -}
> +/* flush writes through the write combiner */
> +#define mb() wr_fence()
> +#define rmb() barrier()
> +#define wmb() mb()
> +
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
>
> #ifndef CONFIG_SMP
> #define fence() do { } while (0)
> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> index cb6d66c..f480097 100644
> --- a/arch/powerpc/include/asm/barrier.h
> +++ b/arch/powerpc/include/asm/barrier.h
> @@ -36,22 +36,20 @@
>
> #define set_mb(var, value) do { var = value; mb(); } while (0)
>
> -#ifdef CONFIG_SMP
> -
> #ifdef __SUBARCH_HAS_LWSYNC
> # define SMPWMB LWSYNC
> #else
> # define SMPWMB eieio
> #endif
>
> -#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> +#define fast_rmb() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> +#define fast_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>
> +#ifdef CONFIG_SMP
> #define smp_mb() mb()
> -#define smp_rmb() __lwsync()
> -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> +#define smp_rmb() fast_rmb()
> +#define smp_wmb() fast_wmb()
> #else
> -#define __lwsync() barrier()
> -
> #define smp_mb() barrier()
> #define smp_rmb() barrier()
> #define smp_wmb() barrier()
> @@ -69,10 +67,16 @@
> #define data_barrier(x) \
> asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
>
> +/*
> + * The use of smp_rmb() in these functions are actually meant to map from
> + * smp_rmb()->fast_rmb()->LWSYNC. This way if smp is disabled then
> + * smp_rmb()->barrier(), or if the platform doesn't support lwsync it will
> + * map to the more heavy-weight sync.
> + */
> #define smp_store_release(p, v) \
> do { \
> compiletime_assert_atomic_type(*p); \
> - __lwsync(); \
> + smp_rmb(); \
> ACCESS_ONCE(*p) = (v); \
> } while (0)
>
> @@ -80,7 +84,7 @@ do { \
> ({ \
> typeof(*p) ___p1 = ACCESS_ONCE(*p); \
> compiletime_assert_atomic_type(*p); \
> - __lwsync(); \
> + smp_rmb(); \
> ___p1; \
> })
>
> diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
> index 33d191d..9a31301 100644
> --- a/arch/s390/include/asm/barrier.h
> +++ b/arch/s390/include/asm/barrier.h
> @@ -24,6 +24,8 @@
>
> #define rmb() mb()
> #define wmb() mb()
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
> #define smp_mb() mb()
> #define smp_rmb() rmb()
> #define smp_wmb() wmb()
> diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
> index 6c974c0..eac2777 100644
> --- a/arch/sparc/include/asm/barrier_64.h
> +++ b/arch/sparc/include/asm/barrier_64.h
> @@ -37,6 +37,9 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
> #define rmb() __asm__ __volatile__("":::"memory")
> #define wmb() __asm__ __volatile__("":::"memory")
>
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
> +
> #define set_mb(__var, __value) \
> do { __var = __value; membar_safe("#StoreLoad"); } while(0)
>
> diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
> index 5238000..cf440e7 100644
> --- a/arch/x86/include/asm/barrier.h
> +++ b/arch/x86/include/asm/barrier.h
> @@ -24,13 +24,16 @@
> #define wmb() asm volatile("sfence" ::: "memory")
> #endif
>
> -#ifdef CONFIG_SMP
> -#define smp_mb() mb()
> #ifdef CONFIG_X86_PPRO_FENCE
> -# define smp_rmb() rmb()
> +#define fast_rmb() rmb()
> #else
> -# define smp_rmb() barrier()
> +#define fast_rmb() barrier()
> #endif
> +#define fast_wmb() barrier()
> +
> +#ifdef CONFIG_SMP
> +#define smp_mb() mb()
> +#define smp_rmb() fast_rmb()
> #define smp_wmb() barrier()
> #define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
> #else /* !SMP */
> diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
> index d6511d9..2c0405d 100644
> --- a/arch/x86/um/asm/barrier.h
> +++ b/arch/x86/um/asm/barrier.h
> @@ -29,17 +29,18 @@
>
> #endif /* CONFIG_X86_32 */
>
> -#ifdef CONFIG_SMP
> -
> -#define smp_mb() mb()
> #ifdef CONFIG_X86_PPRO_FENCE
> -#define smp_rmb() rmb()
> +#define fast_rmb() rmb()
> #else /* CONFIG_X86_PPRO_FENCE */
> -#define smp_rmb() barrier()
> +#define fast_rmb() barrier()
> #endif /* CONFIG_X86_PPRO_FENCE */
> +#define fast_wmb() barrier()
>
> -#define smp_wmb() barrier()
> +#ifdef CONFIG_SMP
>
> +#define smp_mb() mb()
> +#define smp_rmb() fast_rmb()
> +#define smp_wmb() fast_wmb()
> #define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
>
> #else /* CONFIG_SMP */
> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index 1402fa8..c1d60b9 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
> @@ -42,6 +42,14 @@
> #define wmb() mb()
> #endif
>
> +#ifndef fast_rmb
> +#define fast_rmb() rmb()
> +#endif
> +
> +#ifndef fast_wmb
> +#define fast_wmb() wmb()
> +#endif
> +
> #ifndef read_barrier_depends
> #define read_barrier_depends() do { } while (0)
> #endif
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Paul E. McKenney @ 2014-11-17 20:18 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
matthew.vick, geert, jeffrey.t.kirsher, romieu, nic_swsd,
will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
fweisbec, davem
In-Reply-To: <20141117171812.22333.90395.stgit@ahduyck-server>
On Mon, Nov 17, 2014 at 09:18:13AM -0800, Alexander Duyck wrote:
> There are a number of situations where the mandatory barriers rmb() and
> wmb() are used to order memory/memory operations in the device drivers
> and those barriers are much heavier than they actually need to be. For
> example in the case of PowerPC wmb() calls the heavy-weight sync
> instruction when for memory/memory operations all that is really needed is
> an lsync or eieio instruction.
Is this still the case if one of the memory operations is MMIO? Last
I knew, it was not.
> This commit adds a fast (and loose) version of the mandatory memory
> barriers rmb() and wmb(). The prefix to the name is actually based on the
> version of the functions that already exist in the mips and tile trees.
> However I thought it applicable since it gets at what we are trying to
> accomplish with these barriers and somethat implies their risky nature.
>
> These new barriers are not as safe as the standard rmb() and wmb().
> Specifically they do not guarantee ordering between cache-enabled and
> cache-inhibited memories. The primary use case for these would be to
> enforce ordering of memory reads/writes when accessing cache-enabled memory
> that is shared between the CPU and a device.
>
> It may also be noted that there is no fast_mb(). This is due to the fact
> that most architectures didn't seem to have a good way to do a full memory
> barrier quickly and so they usually resorted to an mb() for their smp_mb
> call. As such there no point in adding a fast_mb() function if it is going
> to map to mb() for all architectures anyway.
I must confess that I still don't entirely understand the motivation.
Some problems in PowerPC barrier.h called out below.
Thanx, Paul
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Cc: Michael Ellerman <michael@ellerman.id.au>
> Cc: Michael Neuling <mikey@neuling.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
> Documentation/memory-barriers.txt | 41 +++++++++++++++++++++++++++++++++++
> arch/arm/include/asm/barrier.h | 4 +++
> arch/arm64/include/asm/barrier.h | 3 +++
> arch/ia64/include/asm/barrier.h | 3 +++
> arch/metag/include/asm/barrier.h | 14 ++++++------
> arch/powerpc/include/asm/barrier.h | 22 +++++++++++--------
> arch/s390/include/asm/barrier.h | 2 ++
> arch/sparc/include/asm/barrier_64.h | 3 +++
> arch/x86/include/asm/barrier.h | 11 ++++++---
> arch/x86/um/asm/barrier.h | 13 ++++++-----
> include/asm-generic/barrier.h | 8 +++++++
> 11 files changed, 98 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 22a969c..fe55dea 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
> operations" subsection for information on where to use these.
>
>
> + (*) fast_wmb();
> + (*) fast_rmb();
> +
> + These are for use with memory based device I/O to guarantee the ordering
> + of cache-enabled writes or reads with respect to other writes or reads
> + to cache-enabled memory.
> +
> + For example, consider a device driver that shares memory with a device
> + and uses a descriptor status value to indicate if the descriptor belongs
> + to the device or the CPU, and a doorbell to notify it when new
> + descriptors are available:
> +
> + if (desc->status != DEVICE_OWN) {
> + /* do not read data until we own descriptor */
> + fast_rmb();
> +
> + /* read/modify data */
> + read_data = desc->data;
> + desc->data = write_data;
> +
> + /* flush modifications before status update */
> + fast_wmb();
> +
> + /* assign ownership */
> + desc->status = DEVICE_OWN;
> +
> + /* force memory to sync before notifying device via MMIO */
> + wmb();
> +
> + /* notify device of new descriptors */
> + writel(DESC_NOTIFY, doorbell);
> + }
> +
> + The fast_rmb() allows us guarantee the device has released ownership
> + before we read the data from the descriptor, and he fast_wmb() allows
> + us to guarantee the data is written to the descriptor before the device
> + can see it now has ownership. The wmb() is needed to guarantee that the
> + cache-enabled memory writes have completed before attempting a write to
> + the cache-inhibited MMIO region.
> +
> +
> MMIO WRITE BARRIER
> ------------------
>
> diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
> index c6a3e73..c57903c 100644
> --- a/arch/arm/include/asm/barrier.h
> +++ b/arch/arm/include/asm/barrier.h
> @@ -43,10 +43,14 @@
> #define mb() do { dsb(); outer_sync(); } while (0)
> #define rmb() dsb()
> #define wmb() do { dsb(st); outer_sync(); } while (0)
> +#define fast_rmb() dmb()
> +#define fast_wmb() dmb(st)
> #else
> #define mb() barrier()
> #define rmb() barrier()
> #define wmb() barrier()
> +#define fast_rmb() barrier()
> +#define fast_wmb() barrier()
> #endif
>
> #ifndef CONFIG_SMP
> diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
> index 6389d60..3ca1a15 100644
> --- a/arch/arm64/include/asm/barrier.h
> +++ b/arch/arm64/include/asm/barrier.h
> @@ -32,6 +32,9 @@
> #define rmb() dsb(ld)
> #define wmb() dsb(st)
>
> +#define fast_rmb() dmb(ld)
> +#define fast_wmb() dmb(st)
> +
> #ifndef CONFIG_SMP
> #define smp_mb() barrier()
> #define smp_rmb() barrier()
> diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
> index e8fffb0..997f5b0 100644
> --- a/arch/ia64/include/asm/barrier.h
> +++ b/arch/ia64/include/asm/barrier.h
> @@ -39,6 +39,9 @@
> #define rmb() mb()
> #define wmb() mb()
>
> +#define fast_rmb() mb()
> +#define fast_wmb() mb()
> +
> #ifdef CONFIG_SMP
> # define smp_mb() mb()
> #else
> diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
> index 6d8b8c9..edddb6d 100644
> --- a/arch/metag/include/asm/barrier.h
> +++ b/arch/metag/include/asm/barrier.h
> @@ -4,8 +4,6 @@
> #include <asm/metag_mem.h>
>
> #define nop() asm volatile ("NOP")
> -#define mb() wmb()
> -#define rmb() barrier()
>
> #ifdef CONFIG_METAG_META21
>
> @@ -41,11 +39,13 @@ static inline void wr_fence(void)
>
> #endif /* !CONFIG_METAG_META21 */
>
> -static inline void wmb(void)
> -{
> - /* flush writes through the write combiner */
> - wr_fence();
> -}
> +/* flush writes through the write combiner */
> +#define mb() wr_fence()
> +#define rmb() barrier()
> +#define wmb() mb()
> +
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
>
> #ifndef CONFIG_SMP
> #define fence() do { } while (0)
> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
> index cb6d66c..f480097 100644
> --- a/arch/powerpc/include/asm/barrier.h
> +++ b/arch/powerpc/include/asm/barrier.h
> @@ -36,22 +36,20 @@
>
> #define set_mb(var, value) do { var = value; mb(); } while (0)
>
> -#ifdef CONFIG_SMP
> -
> #ifdef __SUBARCH_HAS_LWSYNC
> # define SMPWMB LWSYNC
> #else
> # define SMPWMB eieio
> #endif
>
> -#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> +#define fast_rmb() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> +#define fast_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>
> +#ifdef CONFIG_SMP
> #define smp_mb() mb()
> -#define smp_rmb() __lwsync()
> -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
> +#define smp_rmb() fast_rmb()
> +#define smp_wmb() fast_wmb()
> #else
> -#define __lwsync() barrier()
> -
> #define smp_mb() barrier()
> #define smp_rmb() barrier()
> #define smp_wmb() barrier()
> @@ -69,10 +67,16 @@
> #define data_barrier(x) \
> asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
>
> +/*
> + * The use of smp_rmb() in these functions are actually meant to map from
> + * smp_rmb()->fast_rmb()->LWSYNC. This way if smp is disabled then
> + * smp_rmb()->barrier(), or if the platform doesn't support lwsync it will
> + * map to the more heavy-weight sync.
> + */
> #define smp_store_release(p, v) \
> do { \
> compiletime_assert_atomic_type(*p); \
> - __lwsync(); \
> + smp_rmb(); \
This is not good at all. For smp_store_release(), we absolutely
must order prior loads and stores against the assignment on the following
line. This is not something that smp_rmb() does, nor is it something
that smp_wmb() does. Yes, it might happen to now, but this could easily
break in the future -- plus this change is extremely misleading.
The original __lwsync() is much more clear.
> ACCESS_ONCE(*p) = (v); \
> } while (0)
>
> @@ -80,7 +84,7 @@ do { \
> ({ \
> typeof(*p) ___p1 = ACCESS_ONCE(*p); \
> compiletime_assert_atomic_type(*p); \
> - __lwsync(); \
> + smp_rmb(); \
> ___p1; \
> })
>
> diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
> index 33d191d..9a31301 100644
> --- a/arch/s390/include/asm/barrier.h
> +++ b/arch/s390/include/asm/barrier.h
> @@ -24,6 +24,8 @@
>
> #define rmb() mb()
> #define wmb() mb()
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
> #define smp_mb() mb()
> #define smp_rmb() rmb()
> #define smp_wmb() wmb()
> diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
> index 6c974c0..eac2777 100644
> --- a/arch/sparc/include/asm/barrier_64.h
> +++ b/arch/sparc/include/asm/barrier_64.h
> @@ -37,6 +37,9 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
> #define rmb() __asm__ __volatile__("":::"memory")
> #define wmb() __asm__ __volatile__("":::"memory")
>
> +#define fast_rmb() rmb()
> +#define fast_wmb() wmb()
> +
> #define set_mb(__var, __value) \
> do { __var = __value; membar_safe("#StoreLoad"); } while(0)
>
> diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
> index 5238000..cf440e7 100644
> --- a/arch/x86/include/asm/barrier.h
> +++ b/arch/x86/include/asm/barrier.h
> @@ -24,13 +24,16 @@
> #define wmb() asm volatile("sfence" ::: "memory")
> #endif
>
> -#ifdef CONFIG_SMP
> -#define smp_mb() mb()
> #ifdef CONFIG_X86_PPRO_FENCE
> -# define smp_rmb() rmb()
> +#define fast_rmb() rmb()
> #else
> -# define smp_rmb() barrier()
> +#define fast_rmb() barrier()
> #endif
> +#define fast_wmb() barrier()
> +
> +#ifdef CONFIG_SMP
> +#define smp_mb() mb()
> +#define smp_rmb() fast_rmb()
> #define smp_wmb() barrier()
> #define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
> #else /* !SMP */
> diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
> index d6511d9..2c0405d 100644
> --- a/arch/x86/um/asm/barrier.h
> +++ b/arch/x86/um/asm/barrier.h
> @@ -29,17 +29,18 @@
>
> #endif /* CONFIG_X86_32 */
>
> -#ifdef CONFIG_SMP
> -
> -#define smp_mb() mb()
> #ifdef CONFIG_X86_PPRO_FENCE
> -#define smp_rmb() rmb()
> +#define fast_rmb() rmb()
> #else /* CONFIG_X86_PPRO_FENCE */
> -#define smp_rmb() barrier()
> +#define fast_rmb() barrier()
> #endif /* CONFIG_X86_PPRO_FENCE */
> +#define fast_wmb() barrier()
>
> -#define smp_wmb() barrier()
> +#ifdef CONFIG_SMP
>
> +#define smp_mb() mb()
> +#define smp_rmb() fast_rmb()
> +#define smp_wmb() fast_wmb()
> #define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
>
> #else /* CONFIG_SMP */
> diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
> index 1402fa8..c1d60b9 100644
> --- a/include/asm-generic/barrier.h
> +++ b/include/asm-generic/barrier.h
> @@ -42,6 +42,14 @@
> #define wmb() mb()
> #endif
>
> +#ifndef fast_rmb
> +#define fast_rmb() rmb()
> +#endif
> +
> +#ifndef fast_wmb
> +#define fast_wmb() wmb()
> +#endif
> +
> #ifndef read_barrier_depends
> #define read_barrier_depends() do { } while (0)
> #endif
>
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 20:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Alexander Duyck
Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz,
heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
matthew.vick, geert, jeffrey.t.kirsher, romieu, paulmck, nic_swsd,
will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
fweisbec, davem
In-Reply-To: <1416254687.18381.3.camel@kernel.crashing.org>
On 11/17/2014 12:04 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2014-11-17 at 09:18 -0800, Alexander Duyck wrote:
>> There are a number of situations where the mandatory barriers rmb() and
>> wmb() are used to order memory/memory operations in the device drivers
>> and those barriers are much heavier than they actually need to be. For
>> example in the case of PowerPC wmb() calls the heavy-weight sync
>> instruction when for memory/memory operations all that is really needed is
>> an lsync or eieio instruction.
> So essentially those are the same as the smp_* variants but not nop'ed
> out on !CONFIG_SMP right ?
>
> Ben.
>
Yes and no. So for example on ARM I used the dmb() operation, however I
have to use the barrier at the system level instead of just the inner
shared domain. However on many other architectures they are just the
same as the smp_* variants.
Basically the resultant code is somewhere between the smp and non-smp
barriers in terms of what they cover.
Thanks,
Alex
^ permalink raw reply
* [PATCH net-next] PPC: bpf_jit_comp: Unify BPF_MOD | BPF_X and BPF_DIV | BPF_X
From: Denis Kirjanov @ 2014-11-17 20:07 UTC (permalink / raw)
To: netdev
Cc: linuxppc-dev, Denis Kirjanov, Alexei Starovoitov, Daniel Borkmann,
Philippe Bergheaud
Reduce duplicated code by unifying
BPF_ALU | BPF_MOD | BPF_X and BPF_ALU | BPF_DIV | BPF_X
CC: Alexei Starovoitov<alexei.starovoitov@gmail.com>
CC: Daniel Borkmann<dborkman@redhat.com>
CC: Philippe Bergheaud<felix@linux.vnet.ibm.com>
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
arch/powerpc/net/bpf_jit_comp.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index d3fa80d..1ca125b 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -181,6 +181,7 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
}
break;
case BPF_ALU | BPF_MOD | BPF_X: /* A %= X; */
+ case BPF_ALU | BPF_DIV | BPF_X: /* A /= X; */
ctx->seen |= SEEN_XREG;
PPC_CMPWI(r_X, 0);
if (ctx->pc_ret0 != -1) {
@@ -190,9 +191,13 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
PPC_LI(r_ret, 0);
PPC_JMP(exit_addr);
}
- PPC_DIVWU(r_scratch1, r_A, r_X);
- PPC_MUL(r_scratch1, r_X, r_scratch1);
- PPC_SUB(r_A, r_A, r_scratch1);
+ if (code == (BPF_ALU | BPF_MOD | BPF_X)) {
+ PPC_DIVWU(r_scratch1, r_A, r_X);
+ PPC_MUL(r_scratch1, r_X, r_scratch1);
+ PPC_SUB(r_A, r_A, r_scratch1);
+ } else {
+ PPC_DIVWU(r_A, r_A, r_X);
+ }
break;
case BPF_ALU | BPF_MOD | BPF_K: /* A %= K; */
PPC_LI32(r_scratch2, K);
@@ -200,22 +205,6 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
PPC_MUL(r_scratch1, r_scratch2, r_scratch1);
PPC_SUB(r_A, r_A, r_scratch1);
break;
- case BPF_ALU | BPF_DIV | BPF_X: /* A /= X; */
- ctx->seen |= SEEN_XREG;
- PPC_CMPWI(r_X, 0);
- if (ctx->pc_ret0 != -1) {
- PPC_BCC(COND_EQ, addrs[ctx->pc_ret0]);
- } else {
- /*
- * Exit, returning 0; first pass hits here
- * (longer worst-case code size).
- */
- PPC_BCC_SHORT(COND_NE, (ctx->idx*4)+12);
- PPC_LI(r_ret, 0);
- PPC_JMP(exit_addr);
- }
- PPC_DIVWU(r_A, r_A, r_X);
- break;
case BPF_ALU | BPF_DIV | BPF_K: /* A /= K */
if (K == 1)
break;
--
2.1.0
^ permalink raw reply related
* [PATCH 1/1 net-next] dccp: kerneldoc warning fixes
From: Fabian Frederick @ 2014-11-17 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/feat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dccp/feat.c b/net/dccp/feat.c
index 9733ddb..1704948 100644
--- a/net/dccp/feat.c
+++ b/net/dccp/feat.c
@@ -478,7 +478,7 @@ static struct dccp_feat_entry *
* @fn_list: feature-negotiation list to update
* @feat: one of %dccp_feature_numbers
* @local: whether local (1) or remote (0) @feat_num is meant
- * @needs_mandatory: whether to use Mandatory feature negotiation options
+ * @mandatory: whether to use Mandatory feature negotiation options
* @fval: pointer to NN/SP value to be inserted (will be copied)
*/
static int dccp_feat_push_change(struct list_head *fn_list, u8 feat, u8 local,
@@ -1050,7 +1050,7 @@ static u8 dccp_feat_prefer(u8 preferred_value, u8 *array, u8 array_len)
/**
* dccp_feat_reconcile - Reconcile SP preference lists
- * @fval: SP list to reconcile into
+ * @fv: SP list to reconcile into
* @arr: received SP preference list
* @len: length of @arr in bytes
* @is_server: whether this side is the server (and @fv is the server's list)
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Linus Torvalds @ 2014-11-17 20:52 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch@vger.kernel.org, Network Development,
Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <20141117171812.22333.90395.stgit@ahduyck-server>
On Mon, Nov 17, 2014 at 9:18 AM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
> There are a number of situations where the mandatory barriers rmb() and
> wmb() are used to order memory/memory operations in the device drivers
> and those barriers are much heavier than they actually need to be.
Ugh. I absolutely despise the name.
It's not "fast". It's just limited. It's the same as "smp_*mb()", in
that it works on cacheable memory, but it actually stays around even
for non-SMP builds.
So I think the name is actively misleading.
Naming should be about what it does, not about some kind of PR thing
that confuses people into thinking it's "better".
Maybe "dma_*mb()" would be acceptable, and ends up having the same
naming convention as "smb_*mb()", and explains what it's about.
And yes, in the same spirit, it would probably be good to try to
eventually get rid of the plain "*mb()" functions, and perhaps call
them "mmio_*mb()" to clarify that they are about ordering memory wrt
mmio.
Hmm?
Linus
^ permalink raw reply
* [PATCH 1/1 net-next] dccp: remove blank lines between function/EXPORT_SYMBOL
From: Fabian Frederick @ 2014-11-17 20:54 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
See Documentation/CodingStyle chapter 6.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/ipv4.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 6ca645c..e45b968 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -140,7 +140,6 @@ failure:
inet->inet_dport = 0;
goto out;
}
-
EXPORT_SYMBOL_GPL(dccp_v4_connect);
/*
@@ -376,7 +375,6 @@ void dccp_v4_send_check(struct sock *sk, struct sk_buff *skb)
inet->inet_saddr,
inet->inet_daddr);
}
-
EXPORT_SYMBOL_GPL(dccp_v4_send_check);
static inline u64 dccp_v4_init_sequence(const struct sk_buff *skb)
@@ -444,7 +442,6 @@ put_and_exit:
dccp_done(newsk);
goto exit;
}
-
EXPORT_SYMBOL_GPL(dccp_v4_request_recv_sock);
static struct sock *dccp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
@@ -670,7 +667,6 @@ drop:
DCCP_INC_STATS_BH(DCCP_MIB_ATTEMPTFAILS);
return -1;
}
-
EXPORT_SYMBOL_GPL(dccp_v4_conn_request);
int dccp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
@@ -729,7 +725,6 @@ discard:
kfree_skb(skb);
return 0;
}
-
EXPORT_SYMBOL_GPL(dccp_v4_do_rcv);
/**
@@ -802,7 +797,6 @@ int dccp_invalid_packet(struct sk_buff *skb)
return 0;
}
-
EXPORT_SYMBOL_GPL(dccp_invalid_packet);
/* this is called when real data arrives */
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] dccp: replace min/casting by min_t
From: Fabian Frederick @ 2014-11-17 20:58 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/ackvec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index ba07824..bd9e718 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -218,7 +218,7 @@ static void dccp_ackvec_add_new(struct dccp_ackvec *av, u32 num_packets,
* different underlying data structure.
*/
for (num_packets = num_cells = 1; lost_packets; ++num_cells) {
- u8 len = min(lost_packets, (u32)DCCPAV_MAX_RUNLEN);
+ u8 len = min_t(u32, lost_packets, DCCPAV_MAX_RUNLEN);
av->av_buf_head = __ackvec_idx_sub(av->av_buf_head, 1);
av->av_buf[av->av_buf_head] = DCCPAV_NOT_RECEIVED | len;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] dccp: spelling s/reseting/resetting
From: Fabian Frederick @ 2014-11-17 21:00 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Gerrit Renker, David S. Miller, dccp, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dccp/input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dccp/input.c b/net/dccp/input.c
index 3c8ec7d..3bd14e8 100644
--- a/net/dccp/input.c
+++ b/net/dccp/input.c
@@ -537,7 +537,7 @@ static int dccp_rcv_respond_partopen_state_process(struct sock *sk,
case DCCP_PKT_DATAACK:
case DCCP_PKT_ACK:
/*
- * FIXME: we should be reseting the PARTOPEN (DELACK) timer
+ * FIXME: we should be resetting the PARTOPEN (DELACK) timer
* here but only if we haven't used the DELACK timer for
* something else, like sending a delayed ack for a TIMESTAMP
* echo, etc, for now were not clearing it, sending an extra
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] net: fix spelling for synchronized
From: Fabian Frederick @ 2014-11-17 21:04 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/core/dev_addr_lists.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index b6b2306..c0548d2 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -278,8 +278,8 @@ int __hw_addr_sync_dev(struct netdev_hw_addr_list *list,
EXPORT_SYMBOL(__hw_addr_sync_dev);
/**
- * __hw_addr_unsync_dev - Remove synchonized addresses from device
- * @list: address list to remove syncronized addresses from
+ * __hw_addr_unsync_dev - Remove synchronized addresses from device
+ * @list: address list to remove synchronized addresses from
* @dev: device to sync
* @unsync: function to call if address should be removed
*
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] i40e: Reduce stack in i40e_dbg_dump_desc
From: David Miller @ 2014-11-17 21:06 UTC (permalink / raw)
To: joe; +Cc: jeffrey.t.kirsher, shannon.nelson, netdev
In-Reply-To: <1416175968.24600.1.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Sun, 16 Nov 2014 14:12:48 -0800
> Reduce stack use by using kmemdup and not using a very
> large struct on stack.
>
> In function ‘i40e_dbg_dump_desc’:
> warning: the frame size of 8192 bytes is larger than 2048 bytes [-Wframe-larger-than=]
>
> Signed-off-by: Joe Perches <joe@perches.com>
I'll let Jeff pick this one up, thanks.
^ permalink raw reply
* [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: Fabian Frederick @ 2014-11-17 21:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/core/link_watch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index bd0767e..49a9e3e 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -21,7 +21,7 @@
#include <linux/spinlock.h>
#include <linux/workqueue.h>
#include <linux/bitops.h>
-#include <asm/types.h>
+#include <linux/types.h>
enum lw_bits {
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] net/core: include linux/uaccess.h instead of asm/uaccess.h
From: Fabian Frederick @ 2014-11-17 21:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/core/gen_estimator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/gen_estimator.c b/net/core/gen_estimator.c
index 9dfb88a..d654d6f 100644
--- a/net/core/gen_estimator.c
+++ b/net/core/gen_estimator.c
@@ -13,7 +13,7 @@
* names to make it usable in general net subsystem.
*/
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
#include <linux/bitops.h>
#include <linux/module.h>
#include <linux/types.h>
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 21:11 UTC (permalink / raw)
To: paulmck
Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
matthew.vick, geert, jeffrey.t.kirsher, romieu, nic_swsd,
will.deacon, michael, tony.luck, torvalds, oleg, schwidefsky,
fweisbec, davem
In-Reply-To: <20141117201823.GD5050@linux.vnet.ibm.com>
On 11/17/2014 12:18 PM, Paul E. McKenney wrote:
> On Mon, Nov 17, 2014 at 09:18:13AM -0800, Alexander Duyck wrote:
>> There are a number of situations where the mandatory barriers rmb() and
>> wmb() are used to order memory/memory operations in the device drivers
>> and those barriers are much heavier than they actually need to be. For
>> example in the case of PowerPC wmb() calls the heavy-weight sync
>> instruction when for memory/memory operations all that is really needed is
>> an lsync or eieio instruction.
>
> Is this still the case if one of the memory operations is MMIO? Last
> I knew, it was not.
This barrier is not meant for use in MMIO operations, for that you still
need a full barrier as I call out in the documentation section. What the
barrier does is allow for a lightweight barrier for accesses to coherent
system memory. So for example many device drivers have to perform a read
of the descriptor to see if the device is done with it. We need an rmb()
following that check to prevent any other accesses.
Right now on x86 that rmb() becomes an lfence instruction and is quite
expensive, and as it turns out we don't need it since the x86 doesn't
reorder reads. The same kind of thing applies to PowerPC, only in that
case we use a sync when what we really need is a lwsync.
>> This commit adds a fast (and loose) version of the mandatory memory
>> barriers rmb() and wmb(). The prefix to the name is actually based on the
>> version of the functions that already exist in the mips and tile trees.
>> However I thought it applicable since it gets at what we are trying to
>> accomplish with these barriers and somethat implies their risky nature.
>>
>> These new barriers are not as safe as the standard rmb() and wmb().
>> Specifically they do not guarantee ordering between cache-enabled and
>> cache-inhibited memories. The primary use case for these would be to
>> enforce ordering of memory reads/writes when accessing cache-enabled memory
>> that is shared between the CPU and a device.
>>
>> It may also be noted that there is no fast_mb(). This is due to the fact
>> that most architectures didn't seem to have a good way to do a full memory
>> barrier quickly and so they usually resorted to an mb() for their smp_mb
>> call. As such there no point in adding a fast_mb() function if it is going
>> to map to mb() for all architectures anyway.
>
> I must confess that I still don't entirely understand the motivation.
The motivation is to provide finer grained barriers. So this provides an
in-between that allows us to "choose the right hammer". In the case of
PowerPC it is the difference between sync/lwsync, on ARM it is
dsb()/dmb(), and on x86 it is lfence/barrier().
> Some problems in PowerPC barrier.h called out below.
>
> Thanx, Paul
>
<snip>
>> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
>> index 22a969c..fe55dea 100644
>> --- a/Documentation/memory-barriers.txt
>> +++ b/Documentation/memory-barriers.txt
>> @@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
>> operations" subsection for information on where to use these.
>>
>>
>> + (*) fast_wmb();
>> + (*) fast_rmb();
>> +
>> + These are for use with memory based device I/O to guarantee the ordering
>> + of cache-enabled writes or reads with respect to other writes or reads
>> + to cache-enabled memory.
>> +
>> + For example, consider a device driver that shares memory with a device
>> + and uses a descriptor status value to indicate if the descriptor belongs
>> + to the device or the CPU, and a doorbell to notify it when new
>> + descriptors are available:
>> +
>> + if (desc->status != DEVICE_OWN) {
>> + /* do not read data until we own descriptor */
>> + fast_rmb();
>> +
>> + /* read/modify data */
>> + read_data = desc->data;
>> + desc->data = write_data;
>> +
>> + /* flush modifications before status update */
>> + fast_wmb();
>> +
>> + /* assign ownership */
>> + desc->status = DEVICE_OWN;
>> +
>> + /* force memory to sync before notifying device via MMIO */
>> + wmb();
>> +
>> + /* notify device of new descriptors */
>> + writel(DESC_NOTIFY, doorbell);
>> + }
>> +
>> + The fast_rmb() allows us guarantee the device has released ownership
>> + before we read the data from the descriptor, and he fast_wmb() allows
>> + us to guarantee the data is written to the descriptor before the device
>> + can see it now has ownership. The wmb() is needed to guarantee that the
>> + cache-enabled memory writes have completed before attempting a write to
>> + the cache-inhibited MMIO region.
>> +
>> +
>> MMIO WRITE BARRIER
>> ------------------
The general idea is that the device/CPU follow acquire/release style
semantics and we need the lightweight barriers to enforce ordering to
prevent us from accessing the descriptors during those periods where we
do not own them. As the example shows we still need a full barrier when
going between MMIO and standard memory. Hopefully that is what is
conveyed in the documentation bits I have above.
<snip>
>> diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
>> index cb6d66c..f480097 100644
>> --- a/arch/powerpc/include/asm/barrier.h
>> +++ b/arch/powerpc/include/asm/barrier.h
>> @@ -36,22 +36,20 @@
>>
>> #define set_mb(var, value) do { var = value; mb(); } while (0)
>>
>> -#ifdef CONFIG_SMP
>> -
>> #ifdef __SUBARCH_HAS_LWSYNC
>> # define SMPWMB LWSYNC
>> #else
>> # define SMPWMB eieio
>> #endif
>>
>> -#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>> +#define fast_rmb() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
>> +#define fast_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>>
>> +#ifdef CONFIG_SMP
>> #define smp_mb() mb()
>> -#define smp_rmb() __lwsync()
>> -#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
>> +#define smp_rmb() fast_rmb()
>> +#define smp_wmb() fast_wmb()
>> #else
>> -#define __lwsync() barrier()
>> -
>> #define smp_mb() barrier()
>> #define smp_rmb() barrier()
>> #define smp_wmb() barrier()
>> @@ -69,10 +67,16 @@
>> #define data_barrier(x) \
>> asm volatile("twi 0,%0,0; isync" : : "r" (x) : "memory");
>>
>> +/*
>> + * The use of smp_rmb() in these functions are actually meant to map from
>> + * smp_rmb()->fast_rmb()->LWSYNC. This way if smp is disabled then
>> + * smp_rmb()->barrier(), or if the platform doesn't support lwsync it will
>> + * map to the more heavy-weight sync.
>> + */
>> #define smp_store_release(p, v) \
>> do { \
>> compiletime_assert_atomic_type(*p); \
>> - __lwsync(); \
>> + smp_rmb(); \
>
> This is not good at all. For smp_store_release(), we absolutely
> must order prior loads and stores against the assignment on the following
> line. This is not something that smp_rmb() does, nor is it something
> that smp_wmb() does. Yes, it might happen to now, but this could easily
> break in the future -- plus this change is extremely misleading.
>
> The original __lwsync() is much more clear.
The problem I had with __lwsync is that it really wasn't all that clear.
It was the lwsync instruction if SMP was enabled, otherwise it was just
a barrier call. What I did is move the definition of __lwsync in the SMP
case into fast_rmb, which in turn is accessed by smp_rmb. I tried to
make this clear in the comment just above the two calls. The resultant
assembly code should be exactly the same.
What I could do is have it added back as a smp_lwsync if that works for
you. That way there is something there to give you a hint that it
becomes a barrier() call as soon as SMP is disabled.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: Eric Dumazet @ 2014-11-17 21:22 UTC (permalink / raw)
To: Fabian Frederick; +Cc: linux-kernel, David S. Miller, netdev
In-Reply-To: <1416258502-9001-1-git-send-email-fabf@skynet.be>
On Mon, 2014-11-17 at 22:08 +0100, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> net/core/link_watch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
> index bd0767e..49a9e3e 100644
> --- a/net/core/link_watch.c
> +++ b/net/core/link_watch.c
> @@ -21,7 +21,7 @@
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> #include <linux/bitops.h>
> -#include <asm/types.h>
> +#include <linux/types.h>
>
We have 99 such occurrences in net/
Are you going to send 99 patches ?
^ permalink raw reply
* [PATCH 1/1 net-next] dev_ioctl: use sizeof(x) instead of sizeof x
From: Fabian Frederick @ 2014-11-17 21:23 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
Also remove spaces after cast.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/core/dev_ioctl.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 72e899a..b94b1d2 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -142,10 +142,12 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
case SIOCGIFHWADDR:
if (!dev->addr_len)
- memset(ifr->ifr_hwaddr.sa_data, 0, sizeof ifr->ifr_hwaddr.sa_data);
+ memset(ifr->ifr_hwaddr.sa_data, 0,
+ sizeof(ifr->ifr_hwaddr.sa_data));
else
memcpy(ifr->ifr_hwaddr.sa_data, dev->dev_addr,
- min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+ min(sizeof(ifr->ifr_hwaddr.sa_data),
+ (size_t)dev->addr_len));
ifr->ifr_hwaddr.sa_family = dev->type;
return 0;
@@ -265,7 +267,8 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
if (ifr->ifr_hwaddr.sa_family != dev->type)
return -EINVAL;
memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
- min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+ min(sizeof(ifr->ifr_hwaddr.sa_data),
+ (size_t)dev->addr_len));
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
return 0;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] i40e: Reduce stack in i40e_dbg_dump_desc
From: Jeff Kirsher @ 2014-11-17 21:30 UTC (permalink / raw)
To: David Miller; +Cc: joe, shannon.nelson, netdev
In-Reply-To: <20141117.160636.429874041980998120.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 578 bytes --]
On Mon, 2014-11-17 at 16:06 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sun, 16 Nov 2014 14:12:48 -0800
>
> > Reduce stack use by using kmemdup and not using a very
> > large struct on stack.
> >
> > In function ‘i40e_dbg_dump_desc’:
> > warning: the frame size of 8192 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
>
> I'll let Jeff pick this one up, thanks.
Consider it picked up. Sorry, trying to catch up on all the
email/patches the went out this weekend.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] fm10k/igb/ixgbe: Use fast_rmb on Rx descriptor reads
From: Jeff Kirsher @ 2014-11-17 21:32 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-arch, netdev, linux-kernel, mathieu.desnoyers, peterz, benh,
heiko.carstens, mingo, mikey, linux, donald.c.skidmore,
matthew.vick, geert, romieu, paulmck, nic_swsd, will.deacon,
michael, tony.luck, torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141117171839.22333.25487.stgit@ahduyck-server>
[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]
On Mon, 2014-11-17 at 09:18 -0800, Alexander Duyck wrote:
> This change makes it so that fast_rmb is used when reading the Rx
> descriptor. The advantage of fast_rmb is that it allows for a much
> lower cost barrier on x86, powerpc, arm, and arm64 architectures than
> a
> traditional memory barrier when dealing with reads that only have to
> synchronize to system memory.
>
> In addition I have updated the code so that it just checks to see if
> any
> bits have been set instead of just the DD bit since the DD bit will
> always
> be set as a part of a descriptor write-back so we just need to check
> for a
> non-zero value being present at that memory location rather than just
> checking for any specific bit. This allows the code itself to appear
> much
> cleaner and allows the compiler more room to optimize.
>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Matthew Vick <matthew.vick@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
> ---
> drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 +++---
> drivers/net/ethernet/intel/igb/igb_main.c | 6 +++---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++-----
> 3 files changed, 10 insertions(+), 11 deletions(-)
Looks like more changes will be coming, based on the feedback on earlier
patches. So I won't be picking this up for validation purposes.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net/core: include linux/types.h instead of asm/types.h
From: David Miller @ 2014-11-17 21:35 UTC (permalink / raw)
To: eric.dumazet; +Cc: fabf, linux-kernel, netdev
In-Reply-To: <1416259376.18588.8.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 17 Nov 2014 13:22:56 -0800
> On Mon, 2014-11-17 at 22:08 +0100, Fabian Frederick wrote:
>> Signed-off-by: Fabian Frederick <fabf@skynet.be>
>> ---
>> net/core/link_watch.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/core/link_watch.c b/net/core/link_watch.c
>> index bd0767e..49a9e3e 100644
>> --- a/net/core/link_watch.c
>> +++ b/net/core/link_watch.c
>> @@ -21,7 +21,7 @@
>> #include <linux/spinlock.h>
>> #include <linux/workqueue.h>
>> #include <linux/bitops.h>
>> -#include <asm/types.h>
>> +#include <linux/types.h>
>>
>
> We have 99 such occurrences in net/
>
> Are you going to send 99 patches ?
Yeah, probably best to consolidate into one or a few patches.
^ permalink raw reply
* Re: [PATCH 2/4] arch: Add lightweight memory barriers fast_rmb() and fast_wmb()
From: Alexander Duyck @ 2014-11-17 21:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch@vger.kernel.org, Network Development,
Linux Kernel Mailing List, Mathieu Desnoyers, Peter Zijlstra,
Benjamin Herrenschmidt, Heiko Carstens, Ingo Molnar,
Michael Neuling, Russell King - ARM Linux, donald.c.skidmore,
matthew.vick, Geert Uytterhoeven, Jeff Kirsher, Francois Romieu,
Paul McKenney, nic_swsd, Will Deacon, Michael Ellerman, Tony Luck,
Oleg Nesterov, Martin Schwidefsky <
In-Reply-To: <CA+55aFxZ9dhB074p3OAQLyQ53TuRjVOo33h6OOda1soh-9xxnQ@mail.gmail.com>
On 11/17/2014 12:52 PM, Linus Torvalds wrote:
> On Mon, Nov 17, 2014 at 9:18 AM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>> There are a number of situations where the mandatory barriers rmb() and
>> wmb() are used to order memory/memory operations in the device drivers
>> and those barriers are much heavier than they actually need to be.
> Ugh. I absolutely despise the name.
>
> It's not "fast". It's just limited. It's the same as "smp_*mb()", in
> that it works on cacheable memory, but it actually stays around even
> for non-SMP builds.
>
> So I think the name is actively misleading.
>
> Naming should be about what it does, not about some kind of PR thing
> that confuses people into thinking it's "better".
>
> Maybe "dma_*mb()" would be acceptable, and ends up having the same
> naming convention as "smb_*mb()", and explains what it's about.
What would you think of the name "coherent_*mb()"? I would prefer to
avoid dma in the name since, at least in my mind, that implies MMIO.
It also ties in well with dma_alloc_coherent/dma_free_coherent which is
what would typically be used to allocate the memory we would be using
the barrier to protect anyway.
> And yes, in the same spirit, it would probably be good to try to
> eventually get rid of the plain "*mb()" functions, and perhaps call
> them "mmio_*mb()" to clarify that they are about ordering memory wrt
> mmio.
>
> Hmm?
>
> Linus
I will work on pulling all of the coherent barrier cases out of using
the plain "*mb()" calls first. We need to sort that out before we could
look at renaming the plain barrier functions.
- Alex
^ permalink raw reply
* Re: [PATCH Iproute2 next v1] ip link: Add ipvlan support to the iproute2/ip util
From: Mahesh Bandewar @ 2014-11-17 21:54 UTC (permalink / raw)
To: Sathya Perla
Cc: netdev, Stephen Hemminger, Eric Dumazet, Maciej Zenczykowski,
Laurent Chavey, Tim Hockin, David Miller, Brandon Philips,
Pavel Emelianov
In-Reply-To: <CF9D1877D81D214CB0CA0669EFAE020C68CEC1DB@CMEXMB1.ad.emulex.com>
On Sun, Nov 16, 2014 at 10:57 PM, Sathya Perla <Sathya.Perla@emulex.com> wrote:
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> >
> > Adding basic support to create virtual devices using 'ip'
> > utility. Following is the syntax -
> >
> > ip link add link <master> <virtual> type ipvlan mode [ l2 | l3 ]
> > e.g. ip link add link eth0 ipvl0 type ipvlan mode l3
> >
> ..
> > ---
> > include/linux/if_link.h | 14 ++++++++
> > ip/Makefile | 2 +-
> > ip/iplink.c | 2 +-
> > ip/iplink_ipvlan.c | 85
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 101 insertions(+), 2 deletions(-)
> > create mode 100644 ip/iplink_ipvlan.c
> >
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 47320636361c..ef1e9f73fb15 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -325,6 +325,20 @@ enum macvlan_macaddr_mode {
> > MACVLAN_MACADDR_SET,
> > };
> >
> > +/* IPVLAN section */
> > +enum {
> > + IFLA_IPVLAN_UNSPEC,
> > + IFLA_IPVLAN_MODE,
> > + __IFLA_IPVLAN_MAX,
> > +};
> > +
> > +#define IFLA_IPVLAN_MAX (__IFLA_IPVLAN_MAX - 1)
> > +
> > +enum ipvlan_mode {
> > + IPVLAN_MODE_L2 = 0, /* Process packets all the way upto L2 */
> > + IPVLAN_MODE_L3 = 1, /* Process Packets all the way upto L3 */
> > +};
> > +
> > #define MACVLAN_FLAG_NOPROMISC 1
> >
> >... +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/socket.h>
> > +#include <linux/if_link.h>
> > +
> > +#include "rt_names.h"
> > +#include "utils.h"
> > +#include "ip_common.h"
> > +
> > +static void explain(void)
> > +{
> > + fprintf(stderr, "Usage: ... ipvlan [ mode { l2 | l3 } ]\n");
> > +}
> > +
> > +static int mode_arg(void)
> > +{
> > + fprintf(stderr, "Error: argument of \"mode\" must be either \"l2\", "
> > + "or \"l3\"\n");
> > + return -1;
> I guess you wanted to "return -1" from the caller routine ipvlan_parse_opt()
> and not from this routine.
>
Hmmm, that caught another error where I intended it to be "return
mode_arg()" instead of just "mode_arg" in the caller. This is clearly
wrong! Thanks for catching it.
> > +}
> > +
> > +static int ipvlan_parse_opt(struct link_util *lu, int argc, char **argv,
> > + struct nlmsghdr *n)
> > +{
> > + while (argc > 0) {
> > + if (matches(*argv, "mode") == 0) {
> > + __u16 mode = 0;
> > + NEXT_ARG();
> > +
> > + if (strcmp(*argv, "l2") == 0)
> > + mode = IPVLAN_MODE_L2;
> > + else if (strcmp(*argv, "l3") == 0)
> > + mode = IPVLAN_MODE_L3;
> > + else
> > + mode_arg();
> > +
> > + addattr16(n, 1024, IFLA_IPVLAN_MODE, mode);
> > + } else if (matches(*argv, "help") == 0) {
> > + explain();
> > + return -1;
> > + } else {
> > + fprintf(stderr, "ipvlan: unknown option \"%s\"?\n",
> > *argv);
> > + explain();
> > + return -1;
> > + }
> > + argc--, argv++;
> > + }
> > +
> > + return 0;
> > +}
> > +
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox