* Re: [PATCH net-next] tun: return NET_XMIT_DROP for dropped packets
From: Jason Wang @ 2014-11-19 3:15 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, netdev, linux-kernel@vger.kernel.org,
Michael S. Tsirkin
In-Reply-To: <CAHA+R7O=vCtLwUPnkVpymxc9kp+a8ChS2MHAJahfJUNUdLX89A@mail.gmail.com>
On 11/19/2014 03:53 AM, Cong Wang wrote:
> On Mon, Nov 17, 2014 at 9:20 PM, Jason Wang <jasowang@redhat.com> wrote:
>> > After commit 5d097109257c03a71845729f8db6b5770c4bbedc
>> > ("tun: only queue packets on device"), NETDEV_TX_OK was returned for
>> > dropped packets. This will confuse pktgen since dropped packets were
>> > counted as sent ones.
>> >
>> > Fixing this by returning NET_XMIT_DROP to let pktgen count it as error
>> > packet.
> pktgen is suspicious, it sends out packets directly without going through
> qdisc, so it should not care about NET_XMIT_* qdisc error code?
Well, NET_XMIT_DROP has been used by some devices. I don't see any side
effect of using this especially consider that pktgen can recognize them.
> Looks like NETDEV_TX_OK doesn't have to mean TX is successful,
> the comment says driver takes care of the packet, can be either dropped
> or sent out. We might need a new code to distinguish success or failure.
Most drivers only drop bad packets when they return NETDEV_TX_OK and
they will stop the txq before tx ring is full. This is not the case of
tun, it never stop txq and keep accepting packets and dropping them when
socket receive queue is full.
^ permalink raw reply
* [PATCH net] cxgb4i : Don't block unload/cxgb4 unload when remote closes TCP connection
From: Anish Bhatt @ 2014-11-19 3:09 UTC (permalink / raw)
To: netdev; +Cc: linux-scsi, davem, hch, jbottomley, kxie, manojmalviya,
Anish Bhatt
cxgb4i was returning wrong error and not releasing module reference if remote
end abruptly closed TCP connection. This prevents the cxgb4 network module from
being unloaded, further affecting other network drivers dependent on cxgb4
Sending to net as this affects all cxgb4 based network drivers.
Signed-off-by: Anish Bhatt <anish@chelsio.com>
---
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c | 2 ++
drivers/scsi/cxgbi/libcxgbi.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index 3e0a0d3..f48f40ce 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -828,6 +828,8 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
if (status == CPL_ERR_RTX_NEG_ADVICE)
goto rel_skb;
+ module_put(THIS_MODULE);
+
if (status && status != CPL_ERR_TCAM_FULL &&
status != CPL_ERR_CONN_EXIST &&
status != CPL_ERR_ARP_MISS)
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 674d498..9d63853 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -816,7 +816,7 @@ static void cxgbi_inform_iscsi_conn_closing(struct cxgbi_sock *csk)
read_lock_bh(&csk->callback_lock);
if (csk->user_data)
iscsi_conn_failure(csk->user_data,
- ISCSI_ERR_CONN_FAILED);
+ ISCSI_ERR_TCP_CONN_CLOSE);
read_unlock_bh(&csk->callback_lock);
}
}
--
2.1.3
^ permalink raw reply related
* {Spam?} Goedemorgen
From: Loans Engine® @ 2014-11-19 2:33 UTC (permalink / raw)
Goedemorgen
Een lening krijgen op 3% op zowel korte als lange termijn. Leningen motor is een globale leningen groep die werd opgericht om te voorzien in de behoeften van economisch depressief cliënten. Wij werken in alle categorieën van leningen. Voer de volgende gegevens als u geïnteresseerd bent.
Volledige naam:
Geslacht:
Land:
Het vereiste bedrag:
Duur:
Missie:
We moeten deze volledige informatie voor lening verwerking.
E-mail ons: loan.engine@outlook.com
Hartelijke groeten
Ana wit
--
Esta mensagem foi verificada pelo sistema de antivirus e
acredita-se estar livre de perigo.
^ permalink raw reply
* Re: [PATCH net-next] tun: return NET_XMIT_DROP for dropped packets
From: Jason Wang @ 2014-11-19 3:09 UTC (permalink / raw)
To: Amos Kong; +Cc: davem, netdev, linux-kernel, Michael S. Tsirkin
In-Reply-To: <20141118165359.GA18636@air.redhat.com>
On 11/19/2014 12:53 AM, Amos Kong wrote:
> On Tue, Nov 18, 2014 at 01:20:41PM +0800, Jason Wang wrote:
>> After commit 5d097109257c03a71845729f8db6b5770c4bbedc
>> ("tun: only queue packets on device"), NETDEV_TX_OK was returned for
>> dropped packets. This will confuse pktgen since dropped packets were
>> counted as sent ones.
>>
>> Fixing this by returning NET_XMIT_DROP to let pktgen count it as error
>> packet.
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/net/tun.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index e3fa65a..ac53a73 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -819,7 +819,7 @@ drop:
>> skb_tx_error(skb);
>> kfree_skb(skb);
>> rcu_read_unlock();
>> - return NETDEV_TX_OK;
>> + return NET_XMIT_DROP;
> Quoted from linux/drivers/firewire/net.c:
>
> /*
> * FIXME: According to a patch from 2003-02-26, "returning non-zero
> * causes serious problems" here, allegedly. Before that patch,
> * -ERRNO was returned which is not appropriate under Linux 2.6.
> * Perhaps more needs to be done? Stop the queue in serious
> * conditions and restart it elsewhere?
> */
>
> I saw many drivers return NETDEV_TX_OK in xmit for drop packets, eg: virtio_net.c
Well, I think you miss some thing:
- Virtio-net only drop packets when there's a bug in either driver or
hypervisor. Other drivers only drop the bad packets. For pktgen, we can
make sure the packet is good.
- Most of the drivers (included virtio-net but not tun) will stop txq
before the ring is full, this could be detected by pktgen
- Tun keep accepting packets and dropping them even if the socket
receive queue is full.
So we really need NET_XMIT_DROP here to let pktgen know the packets were
not sent correctly.
^ permalink raw reply
* Re: [stable request <= 3.11] net/mlx4_en: Fix BlueFlame race
From: Greg KH @ 2014-11-19 1:44 UTC (permalink / raw)
To: Vinson Lee
Cc: stable, cwang, ben, Amir Vadai, Or Gerlitz, Jack Morgenstein,
Eugenia Emantayev, Matan Barak, netdev, David S. Miller
In-Reply-To: <CAHTgTXWcSa0LaGd7+v+8eCLPPQ=ZNkttHuhbhx=ibW1DoDMPCA@mail.gmail.com>
On Wed, Nov 05, 2014 at 11:38:37AM -0800, Vinson Lee wrote:
> On Mon, Nov 3, 2014 at 12:09 PM, David Miller <davem@davemloft.net> wrote:
> > From: Cong Wang <cwang@twopensource.com>
> > Date: Mon, 3 Nov 2014 09:22:18 -0800
> >
> >> On Sat, Nov 1, 2014 at 10:41 AM, David Miller <davem@davemloft.net> wrote:
> >>>
> >>> There is no documented way nor do I wish to state anything so strictly.
> >>> I want maximum flexibility for such a time consuming task.
> >>>
> >>> I tend to go back 3 or 4 releases at most, and it really depends upon
> >>> the difficulty of the backports and my own time constraints.
> >>
> >> You should really offload to developers, otherwise too much work for you. :)
> >
> > That's exactly what I am doing by having the -stable maintainers for older
> > releases deal with the backports and other pains.
>
> + stable
>
> Please backport upstream commit
> 2d4b646613d6b12175b017aca18113945af1faf3 "net/mlx4_en: Fix BlueFlame
> race" to stable kernels. David is no longer supporting 3.10 and older.
>
> Please refer to http://marc.info/?l=linux-netdev&m=141366689811844 for
> previous comments in thread.
Now applied, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH net-net 0/4] Increase the limit of tuntap queues
From: Alexei Starovoitov @ 2014-11-19 1:43 UTC (permalink / raw)
To: Pankaj Gupta
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
David S. Miller, Jason Wang, mst, dgibson, Veaceslav Falico,
Eric Dumazet, Vladislav Yasevich, Jerry Chu, wuzhy,
Pavel Emelianov, Tom Herbert, bhutchings, xii, Stephen Hemminger,
Jiří Pírko, sergei.shtylyov
In-Reply-To: <1416327778-17716-1-git-send-email-pagupta@redhat.com>
On Tue, Nov 18, 2014 at 8:22 AM, Pankaj Gupta <pagupta@redhat.com> wrote:
> Networking under KVM works best if we allocate a per-vCPU rx and tx
> queue in a virtual NIC. This requires a per-vCPU queue on the host side.
...
> I have done some testing to find out any regression and with sample program
> which creates tun/tap for single queue / multiqueue device and it seems to be
> working fine. I will also post the performance numbers.
Sounds quite exciting.
please share recommended setup (queue assignments) and
performance gains you're seeing.
^ permalink raw reply
* Re: Question about Patch Submissions
From: Alexei Starovoitov @ 2014-11-19 1:38 UTC (permalink / raw)
To: rapier; +Cc: Cong Wang, netdev
In-Reply-To: <546BAE75.7080802@psc.edu>
On Tue, Nov 18, 2014 at 12:39 PM, rapier <rapier@psc.edu> wrote:
> break this down for presentation. We can make sure each section of the patch
> applies cleanly but the entirety of the patch set would have to be applied
> for it to compile.
all patches need to compile cleanly on all architectures,
otherwise bisect will be broken.
^ permalink raw reply
* [PATCH v2] ipv6: delete protocol and unregister rtnetlink when cleanup
From: Duan Jiong @ 2014-11-19 1:35 UTC (permalink / raw)
To: David Miller; +Cc: Cong Wang, netdev, eric.dumazet
In-Reply-To: <CAHA+R7PNmac7KQpSK-vO9zVd3+926f6n_GpaGrbpBnSn98xiyQ@mail.gmail.com>
pim6_protocol was added when initiation, but it not deleted.
Similarly, unregister RTNL_FAMILY_IP6MR rtnetlink.
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
v2: add missing rtnl_unregister()
net/ipv6/ip6mr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 0171f08..1a01d79 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1439,6 +1439,10 @@ reg_pernet_fail:
void ip6_mr_cleanup(void)
{
+ rtnl_unregister(RTNL_FAMILY_IP6MR, RTM_GETROUTE);
+#ifdef CONFIG_IPV6_PIMSM_V2
+ inet6_del_protocol(&pim6_protocol, IPPROTO_PIM);
+#endif
unregister_netdevice_notifier(&ip6_mr_notifier);
unregister_pernet_subsys(&ip6mr_net_ops);
kmem_cache_destroy(mrt_cachep);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] bpf: fix arraymap NULL deref and missing overflow and zero size checks
From: Alexei Starovoitov @ 2014-11-19 1:32 UTC (permalink / raw)
To: David S. Miller
Cc: Fengguang Wu, Hannes Frederic Sowa, Daniel Borkmann, netdev,
linux-kernel
- fix NULL pointer dereference:
kernel/bpf/arraymap.c:41 array_map_alloc() error: potential null dereference 'array'. (kzalloc returns null)
kernel/bpf/arraymap.c:41 array_map_alloc() error: we previously assumed 'array' could be null (see line 40)
- integer overflow check was missing in arraymap
(hashmap checks for overflow via kmalloc_array())
- arraymap can round_up(value_size, 8) to zero. check was missing.
- hashmap was missing zero size check as well, since roundup_pow_of_two() can
truncate into zero
- found a typo in the arraymap comment and unnecessary empty line
Fix all of these issues and make both overflow checks explicit U32 in size.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
This silly NULL deref bug and missing overflow check was an oversight when
I refactored the code from two allocations (kmalloc for struct bpf_array and
kcalloc for array of elements) in the first implementation of arraymap
into one allocation which is this code.
kernel/bpf/arraymap.c | 17 +++++++++++------
kernel/bpf/hashtab.c | 5 +++++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 58b80c137afd..9eb4d8a7cd87 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -25,7 +25,7 @@ struct bpf_array {
static struct bpf_map *array_map_alloc(union bpf_attr *attr)
{
struct bpf_array *array;
- u32 elem_size;
+ u32 elem_size, array_size;
/* check sanity of attributes */
if (attr->max_entries == 0 || attr->key_size != 4 ||
@@ -34,11 +34,17 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
elem_size = round_up(attr->value_size, 8);
+ /* check round_up into zero and u32 overflow */
+ if (elem_size == 0 ||
+ attr->max_entries > (U32_MAX - sizeof(*array)) / elem_size)
+ return ERR_PTR(-ENOMEM);
+
+ array_size = sizeof(*array) + attr->max_entries * elem_size;
+
/* allocate all map elements and zero-initialize them */
- array = kzalloc(sizeof(*array) + attr->max_entries * elem_size,
- GFP_USER | __GFP_NOWARN);
+ array = kzalloc(array_size, GFP_USER | __GFP_NOWARN);
if (!array) {
- array = vzalloc(array->map.max_entries * array->elem_size);
+ array = vzalloc(array_size);
if (!array)
return ERR_PTR(-ENOMEM);
}
@@ -51,7 +57,6 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
array->elem_size = elem_size;
return &array->map;
-
}
/* Called from syscall or from eBPF program */
@@ -101,7 +106,7 @@ static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
return -E2BIG;
if (map_flags == BPF_NOEXIST)
- /* all elemenets already exist */
+ /* all elements already exist */
return -EEXIST;
memcpy(array->value + array->elem_size * index, value, array->elem_size);
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index d234a012f046..b3ba43674310 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -65,6 +65,11 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
goto free_htab;
err = -ENOMEM;
+ /* prevent zero size kmalloc and check for u32 overflow */
+ if (htab->n_buckets == 0 ||
+ htab->n_buckets > U32_MAX / sizeof(struct hlist_head))
+ goto free_htab;
+
htab->buckets = kmalloc_array(htab->n_buckets, sizeof(struct hlist_head),
GFP_USER | __GFP_NOWARN);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v5 4/4] fm10k/igb/ixgbe: Use dma_rmb on Rx descriptor reads
From: Jeff Kirsher @ 2014-11-19 1:28 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: <20141119012430.9563.58999.stgit@ahduyck-server>
[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]
On Tue, 2014-11-18 at 17:24 -0800, Alexander Duyck wrote:
> This change makes it so that dma_rmb is used when reading the Rx
> descriptor. The advantage of dma_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 coherent 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(-)
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v5 4/4] fm10k/igb/ixgbe: Use dma_rmb on Rx descriptor reads
From: Alexander Duyck @ 2014-11-19 1:24 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: <20141119012205.9563.95544.stgit@ahduyck-server>
This change makes it so that dma_rmb is used when reading the Rx
descriptor. The advantage of dma_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 coherent 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..06e2d65 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();
+ dma_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..2246f45 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();
+ dma_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..d814b58 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();
+ dma_rmb();
/* retrieve a buffer from the ring */
skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);
^ permalink raw reply related
* [PATCH v5 3/4] r8169: Use dma_rmb() and dma_wmb() for DescOwn checks
From: Alexander Duyck @ 2014-11-19 1:24 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: <20141119012205.9563.95544.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
dma_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
it with a dma_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..39e9796 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 */
+ dma_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 */
+ dma_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
+ */
+ dma_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
+ */
+ dma_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 v5 2/4] arch: Add lightweight memory barriers dma_rmb() and dma_wmb()
From: Alexander Duyck @ 2014-11-19 1:24 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: <20141119012205.9563.95544.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 coherent memory operations all that is really needed
is an lsync or eieio instruction.
This commit adds a coherent only version of the mandatory memory barriers
rmb() and wmb(). In most cases this should result in the barrier being the
same as the SMP barriers for the SMP case, however in some cases we use a
barrier that is somewhere in between rmb() and smp_rmb(). For example on
ARM the rmb barriers break down as follows:
Barrier Call Explanation
--------- -------- ----------------------------------
rmb() dsb() Data synchronization barrier - system
dma_rmb() dmb(osh) data memory barrier - outer sharable
smp_rmb() dmb(ish) data memory barrier - inner sharable
These new barriers are not as safe as the standard rmb() and wmb().
Specifically they do not guarantee ordering between coherent and incoherent
memories. The primary use case for these would be to enforce ordering of
reads and writes when accessing coherent memory that is shared between the
CPU and a device.
It may also be noted that there is no dma_mb(). Most architectures don't
provide a good mechanism for performing a coherent only full barrier without
resorting to the same mechanism used in mb(). As such there isn't much to
be gained in trying to define such a function.
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/mips/include/asm/barrier.h | 9 ++++----
arch/powerpc/include/asm/barrier.h | 13 +++++++----
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 +++++++
12 files changed, 98 insertions(+), 26 deletions(-)
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 22a969c..a1c589b 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.
+ (*) dma_wmb();
+ (*) dma_rmb();
+
+ These are for use with memory based device I/O to guarantee the ordering
+ of cache coherent writes or reads with respect to other writes or reads
+ to cache coherent DMA 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 */
+ dma_rmb();
+
+ /* read/modify data */
+ read_data = desc->data;
+ desc->data = write_data;
+
+ /* flush modifications before status update */
+ dma_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 dma_rmb() allows us guarantee the device has released ownership
+ before we read the data from the descriptor, and he dma_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 coherent memory writes have completed before attempting a write to
+ the cache incoherent MMIO region.
+
+
MMIO WRITE BARRIER
------------------
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
index c6a3e73..d2f81e6 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 dma_rmb() dmb(osh)
+#define dma_wmb() dmb(oshst)
#else
#define mb() barrier()
#define rmb() barrier()
#define wmb() barrier()
+#define dma_rmb() barrier()
+#define dma_wmb() barrier()
#endif
#ifndef CONFIG_SMP
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 6389d60..a5abb00 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 dma_rmb() dmb(oshld)
+#define dma_wmb() dmb(oshst)
+
#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..f6769eb 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 dma_rmb() mb()
+#define dma_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..d703d8e 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 dma_rmb() rmb()
+#define dma_wmb() wmb()
#ifndef CONFIG_SMP
#define fence() do { } while (0)
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index 3d69aa8..2b8bbbc 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -75,20 +75,21 @@
#include <asm/wbflush.h>
-#define wmb() fast_wmb()
-#define rmb() fast_rmb()
#define mb() wbflush()
#define iob() wbflush()
#else /* !CONFIG_CPU_HAS_WB */
-#define wmb() fast_wmb()
-#define rmb() fast_rmb()
#define mb() fast_mb()
#define iob() fast_iob()
#endif /* !CONFIG_CPU_HAS_WB */
+#define wmb() fast_wmb()
+#define rmb() fast_rmb()
+#define dma_wmb() fast_wmb()
+#define dma_rmb() fast_rmb()
+
#if defined(CONFIG_WEAK_ORDERING) && defined(CONFIG_SMP)
# ifdef CONFIG_CPU_CAVIUM_OCTEON
# define smp_mb() __sync()
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index cb6d66c..a3bf5be 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -36,8 +36,6 @@
#define set_mb(var, value) do { var = value; mb(); } while (0)
-#ifdef CONFIG_SMP
-
#ifdef __SUBARCH_HAS_LWSYNC
# define SMPWMB LWSYNC
#else
@@ -45,12 +43,17 @@
#endif
#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
+#define dma_rmb() __lwsync()
+#define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
+
+#ifdef CONFIG_SMP
+#define smp_lwsync() __lwsync()
#define smp_mb() mb()
#define smp_rmb() __lwsync()
#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
#else
-#define __lwsync() barrier()
+#define smp_lwsync() barrier()
#define smp_mb() barrier()
#define smp_rmb() barrier()
@@ -72,7 +75,7 @@
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_lwsync(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
@@ -80,7 +83,7 @@ do { \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_lwsync(); \
___p1; \
})
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index 33d191d..8d72471 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 dma_rmb() rmb()
+#define dma_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..7664894 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 dma_rmb() rmb()
+#define dma_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..2ab1eb3 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 dma_rmb() rmb()
#else
-# define smp_rmb() barrier()
+#define dma_rmb() barrier()
#endif
+#define dma_wmb() barrier()
+
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() dma_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..2d7d9a1 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 dma_rmb() rmb()
#else /* CONFIG_X86_PPRO_FENCE */
-#define smp_rmb() barrier()
+#define dma_rmb() barrier()
#endif /* CONFIG_X86_PPRO_FENCE */
+#define dma_wmb() barrier()
-#define smp_wmb() barrier()
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() dma_rmb()
+#define smp_wmb() barrier()
#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..f5c40b0 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -42,6 +42,14 @@
#define wmb() mb()
#endif
+#ifndef dma_rmb
+#define dma_rmb() rmb()
+#endif
+
+#ifndef dma_wmb
+#define dma_wmb() wmb()
+#endif
+
#ifndef read_barrier_depends
#define read_barrier_depends() do { } while (0)
#endif
^ permalink raw reply related
* [PATCH v5 1/4] arch: Cleanup read_barrier_depends() and comments
From: Alexander Duyck @ 2014-11-19 1:23 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: <20141119012205.9563.95544.stgit@ahduyck-server>
This patch is meant to cleanup the handling of read_barrier_depends and
smp_read_barrier_depends. In multiple spots in the kernel headers
read_barrier_depends is defined as "do {} while (0)", however we then go
into the SMP vs non-SMP sections and have the SMP version reference
read_barrier_depends, and the non-SMP define it as yet another empty
do/while.
With this commit I went through and cleaned out the duplicate definitions
and reduced the number of definitions down to 2 per header. In addition I
moved the 50 line comments for the macro from the x86 and mips headers that
defined it as an empty do/while to those that were actually defining the
macro, alpha and blackfin.
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
arch/alpha/include/asm/barrier.h | 51 ++++++++++++++++++++++++++++++
arch/blackfin/include/asm/barrier.h | 51 ++++++++++++++++++++++++++++++
arch/ia64/include/asm/barrier.h | 22 +++++--------
arch/metag/include/asm/barrier.h | 7 ++--
arch/mips/include/asm/barrier.h | 52 -------------------------------
arch/powerpc/include/asm/barrier.h | 6 ++--
arch/s390/include/asm/barrier.h | 5 ++-
arch/sparc/include/asm/barrier_64.h | 4 +-
arch/x86/include/asm/barrier.h | 59 ++---------------------------------
arch/x86/um/asm/barrier.h | 7 ++--
10 files changed, 129 insertions(+), 135 deletions(-)
diff --git a/arch/alpha/include/asm/barrier.h b/arch/alpha/include/asm/barrier.h
index 3832bdb..77516c8 100644
--- a/arch/alpha/include/asm/barrier.h
+++ b/arch/alpha/include/asm/barrier.h
@@ -7,6 +7,57 @@
#define rmb() __asm__ __volatile__("mb": : :"memory")
#define wmb() __asm__ __volatile__("wmb": : :"memory")
+/**
+ * read_barrier_depends - Flush all pending reads that subsequents reads
+ * depend on.
+ *
+ * No data-dependent reads from memory-like regions are ever reordered
+ * over this barrier. All reads preceding this primitive are guaranteed
+ * to access memory (but not necessarily other CPUs' caches) before any
+ * reads following this primitive that depend on the data return by
+ * any of the preceding reads. This primitive is much lighter weight than
+ * rmb() on most CPUs, and is never heavier weight than is
+ * rmb().
+ *
+ * These ordering constraints are respected by both the local CPU
+ * and the compiler.
+ *
+ * Ordering is not guaranteed by anything other than these primitives,
+ * not even by data dependencies. See the documentation for
+ * memory_barrier() for examples and URLs to more information.
+ *
+ * For example, the following code would force ordering (the initial
+ * value of "a" is zero, "b" is one, and "p" is "&a"):
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * b = 2;
+ * memory_barrier();
+ * p = &b; q = p;
+ * read_barrier_depends();
+ * d = *q;
+ * </programlisting>
+ *
+ * because the read of "*q" depends on the read of "p" and these
+ * two reads are separated by a read_barrier_depends(). However,
+ * the following code, with the same initial values for "a" and "b":
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * a = 2;
+ * memory_barrier();
+ * b = 3; y = b;
+ * read_barrier_depends();
+ * x = a;
+ * </programlisting>
+ *
+ * does not enforce ordering, since there is no data dependency between
+ * the read of "a" and the read of "b". Therefore, on some CPUs, such
+ * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
+ * in cases like this where there are no data dependencies.
+ */
#define read_barrier_depends() __asm__ __volatile__("mb": : :"memory")
#ifdef CONFIG_SMP
diff --git a/arch/blackfin/include/asm/barrier.h b/arch/blackfin/include/asm/barrier.h
index 4200068..dfb66fe 100644
--- a/arch/blackfin/include/asm/barrier.h
+++ b/arch/blackfin/include/asm/barrier.h
@@ -22,6 +22,57 @@
# define mb() do { barrier(); smp_check_barrier(); smp_mark_barrier(); } while (0)
# define rmb() do { barrier(); smp_check_barrier(); } while (0)
# define wmb() do { barrier(); smp_mark_barrier(); } while (0)
+/*
+ * read_barrier_depends - Flush all pending reads that subsequents reads
+ * depend on.
+ *
+ * No data-dependent reads from memory-like regions are ever reordered
+ * over this barrier. All reads preceding this primitive are guaranteed
+ * to access memory (but not necessarily other CPUs' caches) before any
+ * reads following this primitive that depend on the data return by
+ * any of the preceding reads. This primitive is much lighter weight than
+ * rmb() on most CPUs, and is never heavier weight than is
+ * rmb().
+ *
+ * These ordering constraints are respected by both the local CPU
+ * and the compiler.
+ *
+ * Ordering is not guaranteed by anything other than these primitives,
+ * not even by data dependencies. See the documentation for
+ * memory_barrier() for examples and URLs to more information.
+ *
+ * For example, the following code would force ordering (the initial
+ * value of "a" is zero, "b" is one, and "p" is "&a"):
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * b = 2;
+ * memory_barrier();
+ * p = &b; q = p;
+ * read_barrier_depends();
+ * d = *q;
+ * </programlisting>
+ *
+ * because the read of "*q" depends on the read of "p" and these
+ * two reads are separated by a read_barrier_depends(). However,
+ * the following code, with the same initial values for "a" and "b":
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * a = 2;
+ * memory_barrier();
+ * b = 3; y = b;
+ * read_barrier_depends();
+ * x = a;
+ * </programlisting>
+ *
+ * does not enforce ordering, since there is no data dependency between
+ * the read of "a" and the read of "b". Therefore, on some CPUs, such
+ * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
+ * in cases like this where there are no data dependencies.
+ */
# define read_barrier_depends() do { barrier(); smp_check_barrier(); } while (0)
#endif
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index a48957c..e8fffb0 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -35,26 +35,22 @@
* it's (presumably) much slower than mf and (b) mf.a is supported for
* sequential memory pages only.
*/
-#define mb() ia64_mf()
-#define rmb() mb()
-#define wmb() mb()
-#define read_barrier_depends() do { } while(0)
+#define mb() ia64_mf()
+#define rmb() mb()
+#define wmb() mb()
#ifdef CONFIG_SMP
# define smp_mb() mb()
-# define smp_rmb() rmb()
-# define smp_wmb() wmb()
-# define smp_read_barrier_depends() read_barrier_depends()
-
#else
-
# define smp_mb() barrier()
-# define smp_rmb() barrier()
-# define smp_wmb() barrier()
-# define smp_read_barrier_depends() do { } while(0)
-
#endif
+#define smp_rmb() smp_mb()
+#define smp_wmb() smp_mb()
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#define smp_mb__before_atomic() barrier()
#define smp_mb__after_atomic() barrier()
diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
index c7591e8..6d8b8c9 100644
--- a/arch/metag/include/asm/barrier.h
+++ b/arch/metag/include/asm/barrier.h
@@ -47,8 +47,6 @@ static inline void wmb(void)
wr_fence();
}
-#define read_barrier_depends() do { } while (0)
-
#ifndef CONFIG_SMP
#define fence() do { } while (0)
#define smp_mb() barrier()
@@ -82,7 +80,10 @@ static inline void fence(void)
#define smp_wmb() barrier()
#endif
#endif
-#define smp_read_barrier_depends() do { } while (0)
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#define set_mb(var, value) do { var = value; smp_mb(); } while (0)
#define smp_store_release(p, v) \
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index d0101dd..3d69aa8 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -10,58 +10,6 @@
#include <asm/addrspace.h>
-/*
- * read_barrier_depends - Flush all pending reads that subsequents reads
- * depend on.
- *
- * No data-dependent reads from memory-like regions are ever reordered
- * over this barrier. All reads preceding this primitive are guaranteed
- * to access memory (but not necessarily other CPUs' caches) before any
- * reads following this primitive that depend on the data return by
- * any of the preceding reads. This primitive is much lighter weight than
- * rmb() on most CPUs, and is never heavier weight than is
- * rmb().
- *
- * These ordering constraints are respected by both the local CPU
- * and the compiler.
- *
- * Ordering is not guaranteed by anything other than these primitives,
- * not even by data dependencies. See the documentation for
- * memory_barrier() for examples and URLs to more information.
- *
- * For example, the following code would force ordering (the initial
- * value of "a" is zero, "b" is one, and "p" is "&a"):
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * b = 2;
- * memory_barrier();
- * p = &b; q = p;
- * read_barrier_depends();
- * d = *q;
- * </programlisting>
- *
- * because the read of "*q" depends on the read of "p" and these
- * two reads are separated by a read_barrier_depends(). However,
- * the following code, with the same initial values for "a" and "b":
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * a = 2;
- * memory_barrier();
- * b = 3; y = b;
- * read_barrier_depends();
- * x = a;
- * </programlisting>
- *
- * does not enforce ordering, since there is no data dependency between
- * the read of "a" and the read of "b". Therefore, on some CPUs, such
- * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
- * in cases like this where there are no data dependencies.
- */
-
#define read_barrier_depends() do { } while(0)
#define smp_read_barrier_depends() do { } while(0)
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index bab79a1..cb6d66c 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -33,7 +33,6 @@
#define mb() __asm__ __volatile__ ("sync" : : : "memory")
#define rmb() __asm__ __volatile__ ("sync" : : : "memory")
#define wmb() __asm__ __volatile__ ("sync" : : : "memory")
-#define read_barrier_depends() do { } while(0)
#define set_mb(var, value) do { var = value; mb(); } while (0)
@@ -50,16 +49,17 @@
#define smp_mb() mb()
#define smp_rmb() __lwsync()
#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
-#define smp_read_barrier_depends() read_barrier_depends()
#else
#define __lwsync() barrier()
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while(0)
#endif /* CONFIG_SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
/*
* This is a barrier which prevents following instructions from being
* started until the value of the argument x is known. For example, if
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index b5dce65..33d191d 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -24,11 +24,12 @@
#define rmb() mb()
#define wmb() mb()
-#define read_barrier_depends() do { } while(0)
#define smp_mb() mb()
#define smp_rmb() rmb()
#define smp_wmb() wmb()
-#define smp_read_barrier_depends() read_barrier_depends()
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
#define smp_mb__before_atomic() smp_mb()
#define smp_mb__after_atomic() smp_mb()
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 305dcc3..6c974c0 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -37,7 +37,6 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
-#define read_barrier_depends() do { } while(0)
#define set_mb(__var, __value) \
do { __var = __value; membar_safe("#StoreLoad"); } while(0)
@@ -51,7 +50,8 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define smp_wmb() __asm__ __volatile__("":::"memory")
#endif
-#define smp_read_barrier_depends() do { } while(0)
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
#define smp_store_release(p, v) \
do { \
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index 0f4460b..5238000 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -24,60 +24,6 @@
#define wmb() asm volatile("sfence" ::: "memory")
#endif
-/**
- * read_barrier_depends - Flush all pending reads that subsequents reads
- * depend on.
- *
- * No data-dependent reads from memory-like regions are ever reordered
- * over this barrier. All reads preceding this primitive are guaranteed
- * to access memory (but not necessarily other CPUs' caches) before any
- * reads following this primitive that depend on the data return by
- * any of the preceding reads. This primitive is much lighter weight than
- * rmb() on most CPUs, and is never heavier weight than is
- * rmb().
- *
- * These ordering constraints are respected by both the local CPU
- * and the compiler.
- *
- * Ordering is not guaranteed by anything other than these primitives,
- * not even by data dependencies. See the documentation for
- * memory_barrier() for examples and URLs to more information.
- *
- * For example, the following code would force ordering (the initial
- * value of "a" is zero, "b" is one, and "p" is "&a"):
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * b = 2;
- * memory_barrier();
- * p = &b; q = p;
- * read_barrier_depends();
- * d = *q;
- * </programlisting>
- *
- * because the read of "*q" depends on the read of "p" and these
- * two reads are separated by a read_barrier_depends(). However,
- * the following code, with the same initial values for "a" and "b":
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * a = 2;
- * memory_barrier();
- * b = 3; y = b;
- * read_barrier_depends();
- * x = a;
- * </programlisting>
- *
- * does not enforce ordering, since there is no data dependency between
- * the read of "a" and the read of "b". Therefore, on some CPUs, such
- * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
- * in cases like this where there are no data dependencies.
- **/
-
-#define read_barrier_depends() do { } while (0)
-
#ifdef CONFIG_SMP
#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
@@ -86,16 +32,17 @@
# define smp_rmb() barrier()
#endif
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() read_barrier_depends()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* !SMP */
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while (0)
#define set_mb(var, value) do { var = value; barrier(); } while (0)
#endif /* SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#if defined(CONFIG_X86_PPRO_FENCE)
/*
diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
index cc04e67..d6511d9 100644
--- a/arch/x86/um/asm/barrier.h
+++ b/arch/x86/um/asm/barrier.h
@@ -29,8 +29,6 @@
#endif /* CONFIG_X86_32 */
-#define read_barrier_depends() do { } while (0)
-
#ifdef CONFIG_SMP
#define smp_mb() mb()
@@ -42,7 +40,6 @@
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() read_barrier_depends()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* CONFIG_SMP */
@@ -50,11 +47,13 @@
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while (0)
#define set_mb(var, value) do { var = value; barrier(); } while (0)
#endif /* CONFIG_SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
/*
* Stop RDTSC speculation. This is needed when you need to use RDTSC
* (or get_cycles or vread that possibly accesses the TSC) in a defined
^ permalink raw reply related
* [PATCH v5 0/4] arch: Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-19 1:23 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
These patches introduce two new primitives for synchronizing cache coherent
memory writes and reads. These two new primitives are:
dma_rmb()
dma_wmb()
The first patch cleans up some unnecessary overhead related to the
definition of read_barrier_depends, smp_read_barrier_depends, and comments
related to the barrier.
The second patch adds the primitives for the applicable architectures and
asm-generic.
The third patch adds the barriers to r8169 which turns out to be a good
example of where the new barriers might be useful as they have full
rmb()/wmb() barriers ordering accesses to the descriptors and the DescOwn
bit.
The fourth patch adds support for coherent_rmb() to the Intel fm10k, igb,
and ixgbe drivers. Testing with the ixgbe driver has shown a processing
time reduction of at least 7ns per 64B frame on a Core i7-4930K.
This patch series is essentially the v4 for:
v4: Add lightweight memory barriers for coherent memory access
v3: Add lightweight memory barriers fast_rmb() and fast_wmb()
v2: Introduce load_acquire() and store_release()
v1: Introduce read_acquire()
The key changes in this patch series versus the earlier patches are:
v5:
- Renamed barriers dma_rmb and dma_wmb
- Undid smp_wmb changes in x86 and PowerPC
- Defined smp_rmb as __lwsync for SMP case on PowerPC
v4:
- Renamed barriers coherent_rmb and coherent_wmb
- Added smp_lwsync for use in smp_load_acquire/smp_store_release
v3:
- Moved away from acquire()/store() and instead focused on barriers
- Added cleanup of read_barrier_depends
- Added change in r8169 to fix cur_tx/DescOwn ordering
- Simplified changes to just replacing/moving barriers in r8169
- Added update to documentation with code example
v2:
- Renamed read_acquire() to be consistent with smp_load_acquire()
- Changed barrier used to be consistent with smp_load_acquire()
- Updated PowerPC code to use __lwsync based on IBM article
- Added store_release() as this is a viable use case for drivers
- Added r8169 patch which is able to fully use primitives
- Added fm10k/igb/ixgbe patch which is able to test performance
---
Alexander Duyck (4):
arch: Cleanup read_barrier_depends() and comments
arch: Add lightweight memory barriers dma_rmb() and dma_wmb()
r8169: Use dma_rmb() and dma_wmb() for DescOwn checks
fm10k/igb/ixgbe: Use dma_rmb on Rx descriptor reads
Documentation/memory-barriers.txt | 41 +++++++++++++++
arch/alpha/include/asm/barrier.h | 51 ++++++++++++++++++
arch/arm/include/asm/barrier.h | 4 +
arch/arm64/include/asm/barrier.h | 3 +
arch/blackfin/include/asm/barrier.h | 51 ++++++++++++++++++
arch/ia64/include/asm/barrier.h | 25 ++++-----
arch/metag/include/asm/barrier.h | 19 ++++---
arch/mips/include/asm/barrier.h | 61 ++--------------------
arch/powerpc/include/asm/barrier.h | 19 ++++---
arch/s390/include/asm/barrier.h | 7 ++-
arch/sparc/include/asm/barrier_64.h | 7 ++-
arch/x86/include/asm/barrier.h | 70 ++++---------------------
arch/x86/um/asm/barrier.h | 20 ++++---
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 +--
drivers/net/ethernet/realtek/r8169.c | 29 ++++++++--
include/asm-generic/barrier.h | 8 +++
18 files changed, 257 insertions(+), 179 deletions(-)
--
^ permalink raw reply
* Re: [PATCH 3.10] ipv6: reallocate addrconf router for ipv6 address, when lo device up
From: Greg Kroah-Hartman @ 2014-11-19 1:22 UTC (permalink / raw)
To: chenweilong
Cc: David Miller, netdev, stable, Sabrina Dubroca,
Hannes Frederic Sowa, Gao feng, Li Zefan
In-Reply-To: <20141119012155.GC22731@kroah.com>
On Tue, Nov 18, 2014 at 05:21:55PM -0800, Greg Kroah-Hartman wrote:
> On Tue, Aug 12, 2014 at 07:53:46PM +0800, chenweilong wrote:
> > Hi,
> >
> > Please queue this patch for stable 3.10 kernels.
>
> It does not apply, sorry :(
>
Because it's already applied, sorry for the noise...
^ permalink raw reply
* Re: [PATCH 3.10] ipv6: reallocate addrconf router for ipv6 address, when lo device up
From: Greg Kroah-Hartman @ 2014-11-19 1:21 UTC (permalink / raw)
To: chenweilong
Cc: David Miller, netdev, stable, Sabrina Dubroca,
Hannes Frederic Sowa, Gao feng, Li Zefan
In-Reply-To: <53EA004A.9020300@huawei.com>
On Tue, Aug 12, 2014 at 07:53:46PM +0800, chenweilong wrote:
> Hi,
>
> Please queue this patch for stable 3.10 kernels.
It does not apply, sorry :(
^ permalink raw reply
* Re: Missed stable line commit (upstream 859828c0ea476b42f3a93d69d117aaba90994b6f)
From: Greg KH @ 2014-11-19 1:19 UTC (permalink / raw)
To: Andrew Collins; +Cc: netdev@vger.kernel.org, stable
In-Reply-To: <CAKTPYJQQprPnN1bUPxy32YREb8WARQztQYWL9-Qfhm6wLqbD0A@mail.gmail.com>
On Mon, Nov 03, 2014 at 03:11:01PM -0700, Andrew Collins wrote:
> I ran into an issue today on latest 3.10 stable line that looks to
> have been resolved upstream and tagged for stable:
>
> commit 859828c0ea476b42f3a93d69d117aaba90994b6f
> Author: Jiri Pirko <jiri@resnulli.us>
> Date: Thu Dec 5 16:27:37 2013 +0100
>
> br: fix use of ->rx_handler_data in code executed on non-rx_handler path
> ...
> Please apply to stable trees as well. Thanks.
>
> The commit did make it into the 3.12 stable line, but seems to have
> been missed in 3.10. The commit that causes the issue *did* make it
> into 3.10.16 so 3.10 is exposed to the issue:
>
> commit 960b8e5018a552f62cfbc0dfe94be7b6ba178f13
> Author: Hong Zhiguo <zhiguohong@tencent.com>
> Date: Sat Sep 14 22:42:28 2013 +0800
>
> bridge: fix NULL pointer deref of br_port_get_rcu
>
> [ Upstream commit 716ec052d2280d511e10e90ad54a86f5b5d4dcc2 ]
>
> Any chance 859828c0ea476b42f3a93d69d117aaba90994b6f could be pulled
> into the next 3.10 stable?
Now queued up, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH] ipv6: delete protocol when cleanup
From: Duan Jiong @ 2014-11-19 1:10 UTC (permalink / raw)
To: Cong Wang; +Cc: David Miller, netdev
In-Reply-To: <CAHA+R7PNmac7KQpSK-vO9zVd3+926f6n_GpaGrbpBnSn98xiyQ@mail.gmail.com>
On 11/19/2014 02:56 AM, Cong Wang wrote:
> On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
>>
>> pim6_protocol was added when initiation, but it not deleted.
>>
>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>> ---
>> net/ipv6/ip6mr.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
>> index 0171f08..6dcdecd 100644
>> --- a/net/ipv6/ip6mr.c
>> +++ b/net/ipv6/ip6mr.c
>> @@ -1439,6 +1439,9 @@ reg_pernet_fail:
>>
>> void ip6_mr_cleanup(void)
>> {
>> +#ifdef CONFIG_IPV6_PIMSM_V2
>> + inet6_del_protocol(&pim6_protocol, IPPROTO_PIM);
>> +#endif
>> unregister_netdevice_notifier(&ip6_mr_notifier);
>> unregister_pernet_subsys(&ip6mr_net_ops);
>> kmem_cache_destroy(mrt_cachep);
>
>
> Looks like rtnl_unregister() is missing as well.
>
Thanks for pointing this, i will send v2.
Thanks,
Duan
^ permalink raw reply
* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-19 0:07 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+55aFwHUQv+zBxyN41Y4MDyRXbQ84H8zc01O6O_eg9ZvoqYiQ@mail.gmail.com>
On 11/18/2014 03:06 PM, Linus Torvalds wrote:
> On Tue, Nov 18, 2014 at 2:47 PM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>>
>> The problem is DMA is a broad brush. There are multiple cases I can think
>> of where DMA does not represent coherent memory.
>
> .. and I already addressed that, in the thing you even included:
>
>>> about what is actually the important issue. All sane memory is
>>> coherent, after all (and if it isn't, you have other issues than
>>> memory ordering).
>
> The thing is, if the DMA isn't coherent, nobody is going to care about
> the memory barriers anyway. You have bigger issues.
>
> And your argument is that "dma" is bigger than this issue. *MY*
> argument is that "coherent" is bigger than this issue. There's tons of
> coherent memory that is not about DMA, the same way that there is DMA
> memory that isn't coherent.
>
> See? The two are 100% equivalent. Except "dma" is just three letters,
> and matches "smp" both visually and in use (SMP memory is "coherent"
> too - yes, you can - and crap architectures do - have incoherent
> caches due to virtual aliases etc, but exactly as with DMA, if you
> have incoherent SMP, you have bigger issues than the barriers).
Actually if anything maybe the crap architectures are a good reason for
changing the name. If they can't even do coherent SMP memory then the
coherent_*mb() could be misleading since they would just be full
barriers anyway.
> And yes, you could call it "coherent_dma_read_memory_barrier()", and
> it would be very descriptive. It would also drive everybody crazy.
No, I think "dma_wmb__before_coherent_write" would have been much more
descriptive. You have to squeeze in that extra underscore somewhere. ;-)
> So I argue for "dma_mb()" pairing with "smp_mb()" from a naming
> standpoint. It just *describes* the problem better. Look at the
> drivers, it's very much about the devices doing DMA to memory, and our
> ordering.
>
> To be even more clear: nobody sane cares about the "coherent" part,
> because only insane horrible crap architectures have incoherent memory
> in the first place, and sane people run away screaming from that
> steaming pile of sh*t.
I think that is part of my reluctance. I didn't even want it implied
that the barriers could be used with that kind of stuff.
> Just look at some of the drivers you actually *use* this in. They are
> for intel hardware, they presumably would never even work in the first
> place without cache-coherent DMA. Why do you think that "coherent" is
> so important?
>
> Linus
v5 should be up shortly after a quick pass with sed to do the
find/replace, clean up any whitespace issues, and a quick run through
some cross compiling scripts just to make sure I didn't screw anything up.
- Alex
^ permalink raw reply
* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Cong Wang @ 2014-11-18 23:25 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: Xin Long, network dev, Steffen Klassert
In-Reply-To: <546B8093.4080507@6wind.com>
On Tue, Nov 18, 2014 at 9:23 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> A quick look at the ipv6 side seems to show that there is the same problem.
> Can
> you provide the IPv6 patch too?
>
IPv6 doesn't use ip_tunnel library, so needs to provide its own implementation:
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ec84d03..3ae094b 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -911,6 +911,16 @@ static int vti6_newlink(struct net *src_net,
struct net_device *dev,
return vti6_tnl_create2(dev);
}
+static void vti6_dellink(struct net_device *dev, struct list_head *head)
+{
+ struct net *net = dev_net(dev);
+ struct vti6_net *ip6n = net_generic(net, vti6_net_id);
+
+ if (dev != ip6n->fb_tnl_dev)
+ unregister_netdevice_queue(dev, head);
+}
+
+
static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[])
{
@@ -986,6 +996,7 @@ static struct rtnl_link_ops vti6_link_ops __read_mostly = {
.setup = vti6_dev_setup,
.validate = vti6_validate,
.newlink = vti6_newlink,
+ .dellink = vti6_dellink,
.changelink = vti6_changelink,
.get_size = vti6_get_size,
.fill_info = vti6_fill_info,
^ permalink raw reply related
* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Cong Wang @ 2014-11-18 23:19 UTC (permalink / raw)
To: Xin Long; +Cc: network dev
In-Reply-To: <CAHA+R7NKdS_j2MC+-Bcw=hFERODiMw1Z6ju6yw5TvXXHbTXK5A@mail.gmail.com>
On Tue, Nov 18, 2014 at 2:12 PM, Cong Wang <cwang@twopensource.com> wrote:
> On Tue, Nov 18, 2014 at 2:37 AM, Xin Long <lucien.xin@gmail.com> wrote:
>> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
>> the default function, unregister_netdevice_queue, which will cause the
>> dev to unregister later. so when we delete a vti device, the net_device
>> will be removed, but the tunnel still in the tunnel list. then, we add
>> a new vti, in ip_tunnel_find():
>>
>> hlist_for_each_entry_rcu(t, head, hash_node) {
>> if (local == t->parms.iph.saddr &&
>> remote == t->parms.iph.daddr &&
>> link == t->parms.link &&
>> type == t->dev->type &&
>> ip_tunnel_key_match(&t->parms, flags, key))
>> break;
>> }
>>
>> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
>> motioned above. so the panic will happen:
>
>
> Are you sure? We call ->ndo_uninit() in unregister path, and vti tunnel
> has ip_tunnel_uninit() which removes non-fb tunnel from that list.
>
Hmm, your fix is correct, but the description is wrong, we should just skip
the unregister of fb tunnel device like other tunnels. Without ->dellink()
the fb device is still removed from the global netdev list, this needs to skip,
otherwise ip_vti0 will disappear from your system.
^ permalink raw reply
* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Linus Torvalds @ 2014-11-18 23:06 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: <546BCC73.3050903@redhat.com>
On Tue, Nov 18, 2014 at 2:47 PM, Alexander Duyck
<alexander.h.duyck@redhat.com> wrote:
>
> The problem is DMA is a broad brush. There are multiple cases I can think
> of where DMA does not represent coherent memory.
.. and I already addressed that, in the thing you even included:
>> about what is actually the important issue. All sane memory is
>> coherent, after all (and if it isn't, you have other issues than
>> memory ordering).
The thing is, if the DMA isn't coherent, nobody is going to care about
the memory barriers anyway. You have bigger issues.
And your argument is that "dma" is bigger than this issue. *MY*
argument is that "coherent" is bigger than this issue. There's tons of
coherent memory that is not about DMA, the same way that there is DMA
memory that isn't coherent.
See? The two are 100% equivalent. Except "dma" is just three letters,
and matches "smp" both visually and in use (SMP memory is "coherent"
too - yes, you can - and crap architectures do - have incoherent
caches due to virtual aliases etc, but exactly as with DMA, if you
have incoherent SMP, you have bigger issues than the barriers).
And yes, you could call it "coherent_dma_read_memory_barrier()", and
it would be very descriptive. It would also drive everybody crazy.
So I argue for "dma_mb()" pairing with "smp_mb()" from a naming
standpoint. It just *describes* the problem better. Look at the
drivers, it's very much about the devices doing DMA to memory, and our
ordering.
To be even more clear: nobody sane cares about the "coherent" part,
because only insane horrible crap architectures have incoherent memory
in the first place, and sane people run away screaming from that
steaming pile of sh*t.
Just look at some of the drivers you actually *use* this in. They are
for intel hardware, they presumably would never even work in the first
place without cache-coherent DMA. Why do you think that "coherent" is
so important?
Linus
^ permalink raw reply
* Re: [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-18 22:47 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+55aFw3ygP8UEF1vidWdjBUEkqXOELCPkpjLMD-X=vXXQEGOQ@mail.gmail.com>
On 11/18/2014 12:53 PM, Linus Torvalds wrote:
> On Tue, Nov 18, 2014 at 9:28 AM, Alexander Duyck
> <alexander.h.duyck@redhat.com> wrote:
>> These patches introduce two new primitives for synchronizing cache coherent
>> memory writes and reads. These two new primitives are:
>>
>> coherent_rmb()
>> coherent_wmb()
>
> So I'm still not convinced about the name. I don't hate it, but if you
> ever want to do "read_acquire", then that whole "coherent_" thing does
> make for a big mouthful. I don't see why "dma" isn't simpler and more
> to the point, and has the advantage of lining up (in documentation
> etc) with "smp".
The problem is DMA is a broad brush. There are multiple cases I can
think of where DMA does not represent coherent memory.
> Why would you ever use "coherent_xyz()" on something that isn't about
> dma? If it's cache-coherent memory without DMA, you'd use "smp_xyz()",
> so I really do prefer that whole "dma-vs-smp" issue, because it talks
> about what is actually the important issue. All sane memory is
> coherent, after all (and if it isn't, you have other issues than
> memory ordering).
>
> Linus
This barrier only applies to a subset of DMA memory types. So yes,
"coherent_xyz()" only applies to DMA, but not all DMA memory is coherent
as it could be a non-coherent or streaming DMA mapping.
One spot where the name makes sense is in the headers themselves. To
avoid duplication of definitions in several spots if CONFIG_SMP was
defined I simply defined smp_xyz() as coherent_xyz(). Defining it as
dma_xyz() might have made that more difficult to read in terms of what
was going on.
- Alex
^ permalink raw reply
* [PATCH v2] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Jiri Bohac @ 2014-11-18 22:24 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: jbohac, Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141118221057.GA13473@midget.suse.cz>
This fixes an old regression introduced by commit
b0d0d915 (ipx: remove the BKL).
When a recvmsg syscall blocks waiting for new data, no data can be sent on the
same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
This breaks mars-nwe (NetWare emulator):
- the ncpserv process reads the request using recvmsg
- ncpserv forks and spawns nwconn
- ncpserv calls a (blocking) recvmsg and waits for new requests
- nwconn deadlocks in sendmsg on the same socket
Commit b0d0d915 has simply replaced BKL locking with
lock_sock/release_sock. Unlike now, BKL got unlocked while
sleeping, so a blocking recvmsg did not block a concurrent
sendmsg.
Only keep the socket locked while actually working with the socket data and
release it prior to calling skb_recv_datagram().
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 91729b8..1e0d796 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1776,20 +1780,21 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
#ifdef CONFIG_IPX_INTERN
rc = -ENETDOWN;
if (!ipxs->intrfc)
- goto out; /* Someone zonked the iface */
+ goto out_release; /* Someone zonked the iface */
memcpy(uaddr.sipx_node, ipxs->intrfc->if_node, IPX_NODE_LEN);
#endif /* CONFIG_IPX_INTERN */
rc = __ipx_bind(sock, (struct sockaddr *)&uaddr,
sizeof(struct sockaddr_ipx));
if (rc)
- goto out;
+ goto out_release;
}
rc = -ENOTCONN;
if (sock_flag(sk, SOCK_ZAPPED))
- goto out;
+ goto out_release;
+ release_sock(sk);
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &rc);
if (!skb) {
@@ -1807,8 +1812,10 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
rc = skb_copy_datagram_iovec(skb, sizeof(struct ipxhdr), msg->msg_iov,
copied);
- if (rc)
- goto out_free;
+ if (rc) {
+ skb_free_datagram(sk, skb);
+ goto out;
+ }
if (skb->tstamp.tv64)
sk->sk_stamp = skb->tstamp;
@@ -1822,11 +1829,11 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
msg->msg_namelen = sizeof(*sipx);
}
rc = copied;
+ goto out;
-out_free:
- skb_free_datagram(sk, skb);
-out:
+out_release:
release_sock(sk);
+out:
return rc;
}
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
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