* Re: [PATCH v5 3/5] bnx2x: Eliminate duplicate barriers on weakly-ordered archs
From: David Miller @ 2018-03-23 17:16 UTC (permalink / raw)
To: okaya
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
ariel.elior, everest-linux-l2, linux-kernel
In-Reply-To: <b4553d26-b326-15be-dcac-12594ea7c5b5@codeaurora.org>
From: Sinan Kaya <okaya@codeaurora.org>
Date: Fri, 23 Mar 2018 13:13:46 -0400
> We have decided to drop a similar patch on Infiniband due to incorrect
> usage of barrier(). If you feel strongly about it, I can remove the
> DOORBELL change.
Either remove the DOORBELL change or properly adjust the barrier() to
be a wmb().
Be sure to mention this in the commit message, explicitly.
Thank you.
^ permalink raw reply
* Re: [PATCH] dpaa_eth: use true and false for boolean values
From: David Miller @ 2018-03-23 17:18 UTC (permalink / raw)
To: gustavo; +Cc: madalin.bucur, netdev, linux-kernel
In-Reply-To: <20180322195927.GA26854@embeddedgus>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 22 Mar 2018 14:59:27 -0500
> Assign true or false to boolean variables instead of an integer value.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [PATCH] qed: Use true and false for boolean values
From: David Miller @ 2018-03-23 17:18 UTC (permalink / raw)
To: gustavo; +Cc: Ariel.Elior, everest-linux-l2, netdev, linux-kernel
In-Reply-To: <20180322200849.GA28033@embeddedgus>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Thu, 22 Mar 2018 15:08:49 -0500
> Assign true or false to boolean variables instead of an integer value.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied.
^ permalink raw reply
* Re: [RFC PATCH net-next] net: hns3: hclge_inform_reset_assert_to_vf() can be static
From: David Miller @ 2018-03-23 17:18 UTC (permalink / raw)
To: fengguang.wu
Cc: salil.mehta, kbuild-all, netdev, yisen.zhuang, lipeng321,
liangfuyun1, shenjian15, linyunsheng, linux-kernel
In-Reply-To: <20180322213107.GA29844@roam>
From: kbuild test robot <fengguang.wu@intel.com>
Date: Fri, 23 Mar 2018 05:31:07 +0800
>
> Fixes: 2bfbd35d8ecd ("net: hns3: Changes required in PF mailbox to support VF reset")
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Alexey Kodanev @ 2018-03-23 17:13 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: David Miller
In-Reply-To: <538ef522-619b-becd-7391-0770d19fa33d@gmail.com>
On 03/23/2018 06:50 PM, Eric Dumazet wrote:
>
>
> On 03/23/2018 07:39 AM, Alexey Kodanev wrote:
>> After commit 33c162a980fe ("ipv6: datagram: Update dst cache of a
>> connected datagram sk during pmtu update"), when the error occurs on
>> sending datagram in udpv6_sendmsg() due to ICMPV6_PKT_TOOBIG type,
>> error handler can trigger the following path and call ip6_dst_store():
>>
>> udpv6_err()
>> ip6_sk_update_pmtu()
>> ip6_datagram_dst_update()
>> ip6_dst_lookup_flow(), can create a RTF_CACHE clone
>> ...
>> ip6_dst_store()
>>
>> It can happen before a connected UDP socket invokes ip6_dst_store()
>> in the end of udpv6_sendmsg(), on destination release, as a result,
>> the last one changes dst to the old one, preventing getting updated
>> dst cache on the next udpv6_sendmsg() call.
>>
>> This patch moves ip6_dst_store() in udpv6_sendmsg(), so that it is
>> invoked after ip6_sk_dst_lookup_flow() and before udp_v6_send_skb().
>>
>
>
> A Fixes: tag would be nice, for automatic tools (and humans as well)
Hi Eric,
I see, will add the commit 33c162a980fe.
...
>>
>> + if (connected)
>> + ip6_dst_store(sk, dst,
>> + ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
>> + &sk->sk_v6_daddr : NULL,
>> +#ifdef CONFIG_IPV6_SUBTREES
>> + ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
>> + &np->saddr :
>> +#endif
>> + NULL);
>> +
>
> What about the MSG_CONFIRM stuff ?
>
>> if (msg->msg_flags&MSG_CONFIRM)
>> goto do_confirm;
>> back_from_confirm:
>
> Should not you move the above code here instead ?
Ah, you are right, it can release that dst if it go to "do_confirm".
> > Also ip6_dst_store() does not increment dst refcount.
>
> I fear that as soon as dst is visible to other cpus, it might be stolen.
>
So we should pass dst_clone(dst) to ip6_dst_store() instead of dst,
because udpv6_err() can release it if it's set the new one.
Then, I guess, we could left udpv6_sendmsg()/ip6_dst_store() where it
is now in the patch and remove the check for "connected" before dst_relase(),
similar to udp_sendmsg(), right?
Thanks,
Alexey
^ permalink raw reply
* Re: [PATCH 1/2] net: phy: intel-xway: add VR9 version number
From: David Miller @ 2018-03-23 17:20 UTC (permalink / raw)
To: dev; +Cc: andrew, f.fainelli, hauke, netdev, linux-kernel
In-Reply-To: <1521757899-27813-1-git-send-email-dev@kresin.me>
From: Mathias Kresin <dev@kresin.me>
Date: Thu, 22 Mar 2018 23:31:38 +0100
> The VR9 phy ids are matching only for the SoC version 1.2. Rename the
> macros and change the names to take this into account.
>
> Signed-off-by: Mathias Kresin <dev@kresin.me>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] net: phy: intel-xway: add VR9 v1.1 phy ids
From: David Miller @ 2018-03-23 17:20 UTC (permalink / raw)
To: dev; +Cc: andrew, f.fainelli, hauke, netdev, linux-kernel
In-Reply-To: <1521757899-27813-2-git-send-email-dev@kresin.me>
From: Mathias Kresin <dev@kresin.me>
Date: Thu, 22 Mar 2018 23:31:39 +0100
> The phys embedded into the v1.1 of the VR9 SoC are using different phy
> ids. Add the phy ids to use the driver for this VR9 version as well.
>
> Signed-off-by: Mathias Kresin <dev@kresin.me>
Also applied, thank you.
^ permalink raw reply
* Re: [GIT] 'net' merged into 'net-next'
From: Jason Gunthorpe @ 2018-03-23 17:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev, dledford, idosch, dsahern, sd
In-Reply-To: <20180323.114059.297397691681485266.davem@davemloft.net>
On Fri, Mar 23, 2018 at 11:40:59AM -0400, David Miller wrote:
>
> This merge was a little bit more hectic than usual.
>
> But thankfully, I had some sample conflict resolutions to work
> with, in particular for the mlx5 infiniband changes which were
> the most difficult to resolve.
>
> Please double check my work and provide any fixup patches if
> necessary.
The drivers/infiniband looks OK, and I also checked that merging
netdev and rdma together gets us to the right result.
Jason
^ permalink raw reply
* Re: [bpf-next V5 PATCH 14/15] xdp: transition into using xdp_frame for return API
From: Alexander Duyck @ 2018-03-23 17:29 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan
In-Reply-To: <152180755002.20167.11254036418774114364.stgit@firesoul>
On Fri, Mar 23, 2018 at 5:19 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Changing API xdp_return_frame() to take struct xdp_frame as argument,
> seems like a natural choice. But there are some subtle performance
> details here that needs extra care, which is a deliberate choice.
>
> When de-referencing xdp_frame on a remote CPU during DMA-TX
> completion, result in the cache-line is change to "Shared"
> state. Later when the page is reused for RX, then this xdp_frame
> cache-line is written, which change the state to "Modified".
>
> This situation already happens (naturally) for, virtio_net, tun and
> cpumap as the xdp_frame pointer is the queued object. In tun and
> cpumap, the ptr_ring is used for efficiently transferring cache-lines
> (with pointers) between CPUs. Thus, the only option is to
> de-referencing xdp_frame.
>
> It is only the ixgbe driver that had an optimization, in which it can
> avoid doing the de-reference of xdp_frame. The driver already have
> TX-ring queue, which (in case of remote DMA-TX completion) have to be
> transferred between CPUs anyhow. In this data area, we stored a
> struct xdp_mem_info and a data pointer, which allowed us to avoid
> de-referencing xdp_frame.
>
> To compensate for this, a prefetchw is used for telling the cache
> coherency protocol about our access pattern. My benchmarks show that
> this prefetchw is enough to compensate the ixgbe driver.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
I'm really not a fan of this patch. It seems like it is adding a ton
of overhead for one case that is going to penalize all of the other
use cases for XDP. Just the extra prefetch is going to have a
significant impact on things like the XDP_DROP case.
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 +---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 +++++++++++------
> drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 1 +
> drivers/net/tun.c | 4 ++--
> drivers/net/virtio_net.c | 2 +-
> include/net/xdp.h | 2 +-
> kernel/bpf/cpumap.c | 6 +++---
> net/core/xdp.c | 4 +++-
> 8 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index cbc20f199364..dfbc15a45cb4 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -240,8 +240,7 @@ struct ixgbe_tx_buffer {
> unsigned long time_stamp;
> union {
> struct sk_buff *skb;
> - /* XDP uses address ptr on irq_clean */
> - void *data;
> + struct xdp_frame *xdpf;
> };
> unsigned int bytecount;
> unsigned short gso_segs;
> @@ -249,7 +248,6 @@ struct ixgbe_tx_buffer {
> DEFINE_DMA_UNMAP_ADDR(dma);
> DEFINE_DMA_UNMAP_LEN(len);
> u32 tx_flags;
> - struct xdp_mem_info xdp_mem;
> };
>
> struct ixgbe_rx_buffer {
I suppose this makes most of my earlier suggestions pointless, though
I would be interested in seeing what the extra cost is we are taking
for having to initialize one more cache line than we were before to
support the xdp_frame format.
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ff069597fccf..e6e9b28ecfba 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -1207,7 +1207,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
>
> /* free the skb */
> if (ring_is_xdp(tx_ring))
> - xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
> + xdp_return_frame(tx_buffer->xdpf);
> else
> napi_consume_skb(tx_buffer->skb, napi_budget);
>
> @@ -2376,6 +2376,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
> xdp.data_hard_start = xdp.data -
> ixgbe_rx_offset(rx_ring);
> xdp.data_end = xdp.data + size;
> + prefetchw(xdp.data_hard_start); /* xdp_frame write */
>
> skb = ixgbe_run_xdp(adapter, rx_ring, &xdp);
> }
Do we really need to be prefetching yet another cache line? This is
going to hurt general performance since for most cases you are now
prefetching a cache line that will never be used.
> @@ -5787,7 +5788,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring)
>
> /* Free all the Tx ring sk_buffs */
> if (ring_is_xdp(tx_ring))
> - xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem);
> + xdp_return_frame(tx_buffer->xdpf);
> else
> dev_kfree_skb_any(tx_buffer->skb);
>
> @@ -8333,16 +8334,21 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,
> struct ixgbe_ring *ring = adapter->xdp_ring[smp_processor_id()];
> struct ixgbe_tx_buffer *tx_buffer;
> union ixgbe_adv_tx_desc *tx_desc;
> + struct xdp_frame *xdpf;
> u32 len, cmd_type;
> dma_addr_t dma;
> u16 i;
>
> - len = xdp->data_end - xdp->data;
> + xdpf = convert_to_xdp_frame(xdp);
> + if (unlikely(!xdpf))
> + return -EOVERFLOW;
> +
> + len = xdpf->len;
>
> if (unlikely(!ixgbe_desc_unused(ring)))
> return IXGBE_XDP_CONSUMED;
>
> - dma = dma_map_single(ring->dev, xdp->data, len, DMA_TO_DEVICE);
> + dma = dma_map_single(ring->dev, xdpf->data, len, DMA_TO_DEVICE);
> if (dma_mapping_error(ring->dev, dma))
> return IXGBE_XDP_CONSUMED;
>
> @@ -8357,8 +8363,7 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter,
>
> dma_unmap_len_set(tx_buffer, len, len);
> dma_unmap_addr_set(tx_buffer, dma, dma);
> - tx_buffer->data = xdp->data;
> - tx_buffer->xdp_mem = xdp->rxq->mem;
> + tx_buffer->xdpf = xdpf;
>
> tx_desc->read.buffer_addr = cpu_to_le64(dma);
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 2ac78b88fc3d..00b9b13d9fea 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -896,6 +896,7 @@ struct sk_buff *skb_from_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe,
> di->addr + wi->offset,
> 0, frag_size,
> DMA_FROM_DEVICE);
> + prefetchw(va); /* xdp_frame data area */
> prefetch(data);
> wi->offset += frag_size;
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 81fddf9cc58f..a7e42ae1b220 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -663,7 +663,7 @@ static void tun_ptr_free(void *ptr)
> if (tun_is_xdp_frame(ptr)) {
> struct xdp_frame *xdpf = tun_ptr_to_xdp(ptr);
>
> - xdp_return_frame(xdpf->data, &xdpf->mem);
> + xdp_return_frame(xdpf);
> } else {
> __skb_array_destroy_skb(ptr);
> }
> @@ -2188,7 +2188,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
> struct xdp_frame *xdpf = tun_ptr_to_xdp(ptr);
>
> ret = tun_put_user_xdp(tun, tfile, xdpf, to);
> - xdp_return_frame(xdpf->data, &xdpf->mem);
> + xdp_return_frame(xdpf);
> } else {
> struct sk_buff *skb = ptr;
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 48c86accd3b8..479a80339fad 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -430,7 +430,7 @@ static bool __virtnet_xdp_xmit(struct virtnet_info *vi,
>
> /* Free up any pending old buffers before queueing new ones. */
> while ((xdpf_sent = virtqueue_get_buf(sq->vq, &len)) != NULL)
> - xdp_return_frame(xdpf_sent->data, &xdpf_sent->mem);
> + xdp_return_frame(xdpf_sent);
>
> xdpf = convert_to_xdp_frame(xdp);
> if (unlikely(!xdpf))
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 98b55eaf8fd7..35aa9825fdd0 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -103,7 +103,7 @@ struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
> return xdp_frame;
> }
>
> -void xdp_return_frame(void *data, struct xdp_mem_info *mem);
> +void xdp_return_frame(struct xdp_frame *xdpf);
>
> int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq,
> struct net_device *dev, u32 queue_index);
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index bcdc4dea5ce7..c95b04ec103e 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -219,7 +219,7 @@ static void __cpu_map_ring_cleanup(struct ptr_ring *ring)
>
> while ((xdpf = ptr_ring_consume(ring)))
> if (WARN_ON_ONCE(xdpf))
> - xdp_return_frame(xdpf->data, &xdpf->mem);
> + xdp_return_frame(xdpf);
> }
>
> static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
> @@ -275,7 +275,7 @@ static int cpu_map_kthread_run(void *data)
>
> skb = cpu_map_build_skb(rcpu, xdpf);
> if (!skb) {
> - xdp_return_frame(xdpf->data, &xdpf->mem);
> + xdp_return_frame(xdpf);
> continue;
> }
>
> @@ -578,7 +578,7 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
> err = __ptr_ring_produce(q, xdpf);
> if (err) {
> drops++;
> - xdp_return_frame(xdpf->data, &xdpf->mem);
> + xdp_return_frame(xdpf);
> }
> processed++;
> }
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index fe8e87abc266..6ed3d73a73be 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -294,9 +294,11 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> }
> EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
>
> -void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> +void xdp_return_frame(struct xdp_frame *xdpf)
> {
> struct xdp_mem_allocator *xa = NULL;
> + struct xdp_mem_info *mem = &xdpf->mem;
> + void *data = xdpf->data;
>
> rcu_read_lock();
> if (mem->id)
>
^ permalink raw reply
* Re: [PATCH V3 net-next 01/14] tcp: Add clean acked data hook
From: David Miller @ 2018-03-23 17:29 UTC (permalink / raw)
To: saeedm; +Cc: netdev, davejwatson, borisp, ilyal, aviadye
In-Reply-To: <20180322223351.31801-2-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Thu, 22 Mar 2018 15:33:38 -0700
> +#if IS_ENABLED(CONFIG_TLS_DEVICE)
> +DEFINE_STATIC_KEY_FALSE(clean_acked_data_enabled);
> +EXPORT_SYMBOL_GPL(clean_acked_data_enabled);
> +#endif
Please mark this symbol static (otherwise it's name pollutes the global
name space, "clean_acked" what? for what subsystem? etc.)
And add two helper functions, icsk_clean_acked_enable() and
icsk_clean_acked_disable() which adjust the static key as
needed.
For the enable helper, you can pass in an icsk and the function
pointer, so that the helper can do the assignment and the
connection between the static key and registering on an icsk
is clear.
^ permalink raw reply
* Re: [PATCH V3 net-next 06/14] net/tls: Add generic NIC offload infrastructure
From: David Miller @ 2018-03-23 17:31 UTC (permalink / raw)
To: saeedm; +Cc: netdev, davejwatson, borisp, ilyal, aviadye
In-Reply-To: <20180322223351.31801-7-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Thu, 22 Mar 2018 15:33:43 -0700
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> new file mode 100644
> index 000000000000..34555ac0b959
> --- /dev/null
> +++ b/net/tls/tls_device.c
...
> +static inline void tls_append_frag(struct tls_record_info *record,
> + struct page_frag *pfrag,
> + int size)
Do not use the inline keyword in foo.c files, please let the compiler
make that determination.
Please audit this entire submission for this problem.
Thank you.
^ permalink raw reply
* Re: [PATCH net] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Alexey Kodanev @ 2018-03-23 17:43 UTC (permalink / raw)
To: Eric Dumazet, netdev; +Cc: David Miller
In-Reply-To: <6e3bf7fc-572c-9d6f-3d15-85a3b7699f3b@oracle.com>
On 03/23/2018 08:13 PM, Alexey Kodanev wrote:
> On 03/23/2018 06:50 PM, Eric Dumazet wrote:
...
>>> + if (connected)
>>> + ip6_dst_store(sk, dst,
>>> + ipv6_addr_equal(&fl6.daddr, &sk->sk_v6_daddr) ?
>>> + &sk->sk_v6_daddr : NULL,
>>> +#ifdef CONFIG_IPV6_SUBTREES
>>> + ipv6_addr_equal(&fl6.saddr, &np->saddr) ?
>>> + &np->saddr :
>>> +#endif
>>> + NULL);
>>> +
>>
>> What about the MSG_CONFIRM stuff ?
>>
>>> if (msg->msg_flags&MSG_CONFIRM)
>>> goto do_confirm;
>>> back_from_confirm:
>>
>> Should not you move the above code here instead ?
>
> Ah, you are right, it can release that dst if it go to "do_confirm".
>
>>> Also ip6_dst_store() does not increment dst refcount.
>>
>> I fear that as soon as dst is visible to other cpus, it might be stolen.
>>
>
> So we should pass dst_clone(dst) to ip6_dst_store() instead of dst,
> because udpv6_err() can release it if it's set the new one.
>
> Then, I guess, we could left udpv6_sendmsg()/ip6_dst_store() where it
> is now in the patch and remove the check for "connected" before dst_relase(),
> similar to udp_sendmsg(), right?
And the section "release_dst:" looks redundant after that too.
Thanks,
Alexey
^ permalink raw reply
* Re: [GIT] 'net' merged into 'net-next'
From: David Miller @ 2018-03-23 17:36 UTC (permalink / raw)
To: jgg; +Cc: netdev, dledford, idosch, dsahern, sd
In-Reply-To: <20180323172622.GC13003@mellanox.com>
From: Jason Gunthorpe <jgg@mellanox.com>
Date: Fri, 23 Mar 2018 11:26:22 -0600
> On Fri, Mar 23, 2018 at 11:40:59AM -0400, David Miller wrote:
>>
>> This merge was a little bit more hectic than usual.
>>
>> But thankfully, I had some sample conflict resolutions to work
>> with, in particular for the mlx5 infiniband changes which were
>> the most difficult to resolve.
>>
>> Please double check my work and provide any fixup patches if
>> necessary.
>
> The drivers/infiniband looks OK, and I also checked that merging
> netdev and rdma together gets us to the right result.
Thanks for looking at it.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix possible deadlock in rt6_age_examine_exception()
From: David Miller @ 2018-03-23 17:41 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, weiwan, kafai
In-Reply-To: <20180323145658.154636-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 23 Mar 2018 07:56:58 -0700
> syzbot reported a LOCKDEP splat [1] in rt6_age_examine_exception()
>
> rt6_age_examine_exception() is called while rt6_exception_lock is held.
> This lock is the lower one in the lock hierarchy, thus we can not
> call dst_neigh_lookup() function, as it can fallback to neigh_create()
>
> We should instead do a pure RCU lookup. As a bonus we avoid
> a pair of atomic operations on neigh refcount.
>
> [1]
...
> Fixes: c757faa8bfa2 ("ipv6: prepare fib6_age() for exception table")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* [PATCH net-next] mlxsw: spectrum_span: Prevent duplicate mirrors
From: Ido Schimmel @ 2018-03-23 18:03 UTC (permalink / raw)
To: netdev; +Cc: davem, petrm, jiri, mlxsw, Ido Schimmel
In net commit 8175f7c4736f ("mlxsw: spectrum: Prevent duplicate
mirrors") we prevented the user from mirroring more than once from a
single binding point (port-direction pair).
The fix was essentially reverted in a merge conflict resolution when net
was merged into net-next. Restore it.
Fixes: 03fe2debbb27 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_span.c | 28 ++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index ac24e52d74db..65a77708ff61 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -600,13 +600,17 @@ int mlxsw_sp_span_port_mtu_update(struct mlxsw_sp_port *port, u16 mtu)
}
static struct mlxsw_sp_span_inspected_port *
-mlxsw_sp_span_entry_bound_port_find(struct mlxsw_sp_port *port,
- struct mlxsw_sp_span_entry *span_entry)
+mlxsw_sp_span_entry_bound_port_find(struct mlxsw_sp_span_entry *span_entry,
+ enum mlxsw_sp_span_type type,
+ struct mlxsw_sp_port *port,
+ bool bind)
{
struct mlxsw_sp_span_inspected_port *p;
list_for_each_entry(p, &span_entry->bound_ports_list, list)
- if (port->local_port == p->local_port)
+ if (type == p->type &&
+ port->local_port == p->local_port &&
+ bind == p->bound)
return p;
return NULL;
}
@@ -636,8 +640,22 @@ mlxsw_sp_span_inspected_port_add(struct mlxsw_sp_port *port,
struct mlxsw_sp_span_inspected_port *inspected_port;
struct mlxsw_sp *mlxsw_sp = port->mlxsw_sp;
char sbib_pl[MLXSW_REG_SBIB_LEN];
+ int i;
int err;
+ /* A given (source port, direction) can only be bound to one analyzer,
+ * so if a binding is requested, check for conflicts.
+ */
+ if (bind)
+ for (i = 0; i < mlxsw_sp->span.entries_count; i++) {
+ struct mlxsw_sp_span_entry *curr =
+ &mlxsw_sp->span.entries[i];
+
+ if (mlxsw_sp_span_entry_bound_port_find(curr, type,
+ port, bind))
+ return -EEXIST;
+ }
+
/* if it is an egress SPAN, bind a shared buffer to it */
if (type == MLXSW_SP_SPAN_EGRESS) {
u32 buffsize = mlxsw_sp_span_mtu_to_buffsize(mlxsw_sp,
@@ -665,6 +683,7 @@ mlxsw_sp_span_inspected_port_add(struct mlxsw_sp_port *port,
}
inspected_port->local_port = port->local_port;
inspected_port->type = type;
+ inspected_port->bound = bind;
list_add_tail(&inspected_port->list, &span_entry->bound_ports_list);
return 0;
@@ -691,7 +710,8 @@ mlxsw_sp_span_inspected_port_del(struct mlxsw_sp_port *port,
struct mlxsw_sp *mlxsw_sp = port->mlxsw_sp;
char sbib_pl[MLXSW_REG_SBIB_LEN];
- inspected_port = mlxsw_sp_span_entry_bound_port_find(port, span_entry);
+ inspected_port = mlxsw_sp_span_entry_bound_port_find(span_entry, type,
+ port, bind);
if (!inspected_port)
return;
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL
From: Christoph Hellwig @ 2018-03-23 18:05 UTC (permalink / raw)
To: Al Viro
Cc: Christoph Hellwig, Avi Kivity, linux-aio, linux-fsdevel, netdev,
linux-api, linux-kernel
In-Reply-To: <20180322181653.GJ30522@ZenIV.linux.org.uk>
On Thu, Mar 22, 2018 at 06:16:53PM +0000, Al Viro wrote:
> On Thu, Mar 22, 2018 at 06:24:10PM +0100, Christoph Hellwig wrote:
>
> > -static void aio_complete(struct aio_kiocb *iocb, long res, long res2)
> > +static bool aio_complete(struct aio_kiocb *iocb, long res, long res2,
> > + unsigned complete_flags)
>
> Looks like all callers are following that with "if returned true,
> fput(something)". Does it really make any sense to keep that struct
> file * in different fields?
struct kiocb is used not just for aio, but for our normal read/write_iter
APIs, and it is not suitable for poll or fsync. So I can't really find
a good way to keep it common except for duplicating it in struct kiocb
and strut aio_iocb. But maybe we could pass a struct file argument
to aio_complete().
> Wait a sec... What ordering do we want for
> * call(s) of ->ki_complete
> * call (if any) of ->ki_cancel
> * dropping reference to struct file
> and what are the expected call chains for all of those?
fput must be done exactly once from inside ->ki_complete OR ->ki_cancel
in case it did manage to do the actual completion. Reference to struct
file isn't needed in aio_complete, but if aio_complete decided who
won the race we'll have to put after it (or inside it if we want to make
it common)
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [GIT] 'net' merged into 'net-next'
From: Ido Schimmel @ 2018-03-23 18:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jgg, dledford, dsahern, sd
In-Reply-To: <20180323.114059.297397691681485266.davem@davemloft.net>
On Fri, Mar 23, 2018 at 11:40:59AM -0400, David Miller wrote:
>
> This merge was a little bit more hectic than usual.
>
> But thankfully, I had some sample conflict resolutions to work
> with, in particular for the mlx5 infiniband changes which were
> the most difficult to resolve.
>
> Please double check my work and provide any fixup patches if
> necessary.
Fixup for mlxsw:
http://patchwork.ozlabs.org/patch/890093/
^ permalink raw reply
* [PATCH net-next] net/sched: remove tcf_idr_cleanup()
From: Davide Caratti @ 2018-03-23 18:09 UTC (permalink / raw)
To: Cong Wang, David S. Miller; +Cc: netdev
tcf_idr_cleanup() is no more used, so remove it.
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/net/act_api.h | 1 -
net/sched/act_api.c | 8 --------
2 files changed, 9 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index e0a9c2003b24..9e59ebfded62 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -149,7 +149,6 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
struct tc_action **a, const struct tc_action_ops *ops,
int bind, bool cpustats);
-void tcf_idr_cleanup(struct tc_action *a, struct nlattr *est);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 57cf37145282..7bd1b964f021 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -296,14 +296,6 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
}
EXPORT_SYMBOL(tcf_idr_check);
-void tcf_idr_cleanup(struct tc_action *a, struct nlattr *est)
-{
- if (est)
- gen_kill_estimator(&a->tcfa_rate_est);
- free_tcf(a);
-}
-EXPORT_SYMBOL(tcf_idr_cleanup);
-
int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
struct tc_action **a, const struct tc_action_ops *ops,
int bind, bool cpustats)
--
2.14.3
^ permalink raw reply related
* Re: [bpf-next V5 PATCH 10/15] xdp: rhashtable with allocator ID to pointer mapping
From: Jesper Dangaard Brouer @ 2018-03-23 18:15 UTC (permalink / raw)
To: Alexander Duyck
Cc: Netdev, BjörnTöpel, Karlsson, Magnus, Eugenia Emantayev,
Jason Wang, John Fastabend, Eran Ben Elisha, Saeed Mahameed,
Gal Pressman, Daniel Borkmann, Alexei Starovoitov, Tariq Toukan,
brouer
In-Reply-To: <CAKgT0Ucmouu0B1Hxfk2x-2cbbTqqanf43-RDMPdcEmSFaUACzQ@mail.gmail.com>
On Fri, 23 Mar 2018 09:56:50 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:
> On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > Use the IDA infrastructure for getting a cyclic increasing ID number,
> > that is used for keeping track of each registered allocator per
> > RX-queue xdp_rxq_info. Instead of using the IDR infrastructure, which
> > uses a radix tree, use a dynamic rhashtable, for creating ID to
> > pointer lookup table, because this is faster.
> >
> > The problem that is being solved here is that, the xdp_rxq_info
> > pointer (stored in xdp_buff) cannot be used directly, as the
> > guaranteed lifetime is too short. The info is needed on a
> > (potentially) remote CPU during DMA-TX completion time . In an
> > xdp_frame the xdp_mem_info is stored, when it got converted from an
> > xdp_buff, which is sufficient for the simple page refcnt based recycle
> > schemes.
> >
> > For more advanced allocators there is a need to store a pointer to the
> > registered allocator. Thus, there is a need to guard the lifetime or
> > validity of the allocator pointer, which is done through this
> > rhashtable ID map to pointer. The removal and validity of of the
> > allocator and helper struct xdp_mem_allocator is guarded by RCU. The
> > allocator will be created by the driver, and registered with
> > xdp_rxq_info_reg_mem_model().
> >
> > It is up-to debate who is responsible for freeing the allocator
> > pointer or invoking the allocator destructor function. In any case,
> > this must happen via RCU freeing.
> >
> > Use the IDA infrastructure for getting a cyclic increasing ID number,
> > that is used for keeping track of each registered allocator per
> > RX-queue xdp_rxq_info.
> >
> > V4: Per req of Jason Wang
> > - Use xdp_rxq_info_reg_mem_model() in all drivers implementing
> > XDP_REDIRECT, even-though it's not strictly necessary when
> > allocator==NULL for type MEM_TYPE_PAGE_SHARED (given it's zero).
> >
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 +
> > drivers/net/tun.c | 6 +
> > drivers/net/virtio_net.c | 7 +
> > include/net/xdp.h | 15 --
> > net/core/xdp.c | 230 ++++++++++++++++++++++++-
> > 5 files changed, 248 insertions(+), 19 deletions(-)
> >
[...]
> > int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> > enum mem_type type, void *allocator)
> > {
> > + struct xdp_mem_allocator *xdp_alloc;
> > + gfp_t gfp = GFP_KERNEL;
> > + int id, errno, ret;
> > + void *ptr;
> > +
> > + if (xdp_rxq->reg_state != REG_STATE_REGISTERED) {
> > + WARN(1, "Missing register, driver bug");
> > + return -EFAULT;
> > + }
> > +
> > if (type >= MEM_TYPE_MAX)
> > return -EINVAL;
> >
> > xdp_rxq->mem.type = type;
> >
> > - if (allocator)
> > - return -EOPNOTSUPP;
> > + if (!allocator)
> > + return 0;
> > +
> > + /* Delay init of rhashtable to save memory if feature isn't used */
> > + if (!mem_id_init) {
> > + mutex_lock(&mem_id_lock);
> > + ret = __mem_id_init_hash_table();
> > + mutex_unlock(&mem_id_lock);
> > + if (ret < 0) {
> > + WARN_ON(1);
> > + return ret;
> > + }
> > + }
> > +
> > + xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
> > + if (!xdp_alloc)
> > + return -ENOMEM;
> > +
> > + mutex_lock(&mem_id_lock);
> > + id = __mem_id_cyclic_get(gfp);
> > + if (id < 0) {
> > + errno = id;
> > + goto err;
> > + }
> > + xdp_rxq->mem.id = id;
> > + xdp_alloc->mem = xdp_rxq->mem;
> > + xdp_alloc->allocator = allocator;
> > +
> > + /* Insert allocator into ID lookup table */
> > + ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
> > + if (IS_ERR(ptr)) {
> > + errno = PTR_ERR(ptr);
> > + goto err;
> > + }
> > +
> > + mutex_unlock(&mem_id_lock);
> >
> > - /* TODO: Allocate an ID that maps to allocator pointer
> > - * See: https://www.kernel.org/doc/html/latest/core-api/idr.html
> > - */
> > return 0;
> > +err:
> > + mutex_unlock(&mem_id_lock);
> > + kfree(xdp_alloc);
> > + return errno;
> > }
> > EXPORT_SYMBOL_GPL(xdp_rxq_info_reg_mem_model);
> > +
> > +void xdp_return_frame(void *data, struct xdp_mem_info *mem)
> > +{
> > + struct xdp_mem_allocator *xa;
> > +
> > + rcu_read_lock();
> > + if (mem->id)
> > + xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > + rcu_read_unlock();
> > +
> > + if (mem->type == MEM_TYPE_PAGE_SHARED) {
> > + page_frag_free(data);
> > + return;
> > + }
> > +
> > + if (mem->type == MEM_TYPE_PAGE_ORDER0) {
> > + struct page *page = virt_to_page(data); /* Assumes order0 page*/
> > +
> > + put_page(page);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(xdp_return_frame);
> >
>
> I'm not sure what the point is of getting the xa value if it is not
> going to be used. Also I would assume there are types that won't even
> need the hash table lookup. I would prefer to see this bit held off on
> until you have something that actually needs it.
I think, you misread the patch. The lookup is NOT going to be performed
when mem->id is zero, which is the case that you are interested in for
your ixgbe driver.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH v6 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: sulrich, netdev, timur, Sinan Kaya, linux-arm-msm,
linux-arm-kernel
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
I did a regex search for wmb() followed by writel() in each drivers
directory.
I scrubbed the ones I care about in this series.
I considered "ease of change", "popular usage" and "performance critical
path" as the determining criteria for my filtering.
We used relaxed API heavily on ARM for a long time but
it did not exist on other architectures. For this reason, relaxed
architectures have been paying double penalty in order to use the common
drivers.
Now that relaxed API is present on all architectures, we can go and scrub
all drivers to see what needs to change and what can remain.
We start with mostly used ones and hope to increase the coverage over time.
It will take a while to cover all drivers.
Feel free to apply patches individually.
Changes since v5:
add mmiowb to missing places in order not to break PPC
Changes since v4:
posted ixgbevf: keep writel() closer to wmb() to jkircher
posted ixgbevf: eliminate duplicate barriers on weakly-ordered archs to
Sinan Kaya (7):
i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs
ixgbe: eliminate duplicate barriers on weakly-ordered archs
igbvf: eliminate duplicate barriers on weakly-ordered archs
igb: eliminate duplicate barriers on weakly-ordered archs
fm10k: Eliminate duplicate barriers on weakly-ordered archs
ixgbevf: keep writel() closer to wmb()
ixgbevf: eliminate duplicate barriers on weakly-ordered archs
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 9 +++++++--
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 24 +++++++++++++++++++----
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 9 +++++++--
drivers/net/ethernet/intel/igb/igb_main.c | 9 +++++++--
drivers/net/ethernet/intel/igbvf/netdev.c | 9 +++++++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 ++++++++++++++++++----
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 5 -----
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 21 +++++++++++++++++---
8 files changed, 85 insertions(+), 24 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v6 1/7] i40e/i40evf: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
Sinan Kaya, intel-wired-lan, linux-kernel
In-Reply-To: <1521829277-9398-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier
on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 24 ++++++++++++++++++++----
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 9 +++++++--
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c6972bd..fc10cc0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -186,7 +186,13 @@ static int i40e_program_fdir_filter(struct i40e_fdir_filter *fdir_data,
/* Mark the data descriptor to be watched */
first->next_to_watch = tx_desc;
- writel(tx_ring->next_to_use, tx_ring->tail);
+ writel_relaxed(tx_ring->next_to_use, tx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
+
return 0;
dma_fail:
@@ -1529,7 +1535,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
* such as IA-64).
*/
wmb();
- writel(val, rx_ring->tail);
+ writel_relaxed(val, rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
/**
@@ -2412,7 +2423,12 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)
*/
wmb();
- writel(xdp_ring->next_to_use, xdp_ring->tail);
+ writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
rx_ring->skb = skb;
@@ -3437,7 +3453,7 @@ static inline int i40e_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
/* notify HW of packet */
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- writel(i, tx_ring->tail);
+ writel_relaxed(i, tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 1ae112f..ca02762 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -810,7 +810,12 @@ static inline void i40e_release_rx_desc(struct i40e_ring *rx_ring, u32 val)
* such as IA-64).
*/
wmb();
- writel(val, rx_ring->tail);
+ writel_relaxed(val, rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
/**
@@ -2379,7 +2384,7 @@ static inline void i40evf_tx_map(struct i40e_ring *tx_ring, struct sk_buff *skb,
/* notify HW of packet */
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- writel(i, tx_ring->tail);
+ writel_relaxed(i, tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
--
2.7.4
^ permalink raw reply related
* [PATCH v6 2/7] ixgbe: eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
Sinan Kaya, intel-wired-lan, linux-kernel
In-Reply-To: <1521829277-9398-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e3b32ea..1ecc2f5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1701,7 +1701,12 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
* such as IA-64).
*/
wmb();
- writel(i, rx_ring->tail);
+ writel_relaxed(i, rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
}
@@ -2470,7 +2475,12 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
* know there are new descriptors to fetch.
*/
wmb();
- writel(ring->next_to_use, ring->tail);
+ writel_relaxed(ring->next_to_use, ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
xdp_do_flush_map();
}
@@ -8101,7 +8111,7 @@ static int ixgbe_tx_map(struct ixgbe_ring *tx_ring,
ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- writel(i, tx_ring->tail);
+ writel_relaxed(i, tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
@@ -10038,7 +10048,12 @@ static void ixgbe_xdp_flush(struct net_device *dev)
* are new descriptors to fetch.
*/
wmb();
- writel(ring->next_to_use, ring->tail);
+ writel_relaxed(ring->next_to_use, ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
return;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v6 3/7] igbvf: eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: sulrich, netdev, timur, linux-kernel, Sinan Kaya, intel-wired-lan,
linux-arm-msm, linux-arm-kernel
In-Reply-To: <1521829277-9398-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier
on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/intel/igbvf/netdev.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index fa07876..6dfd3dc 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -252,7 +252,12 @@ static void igbvf_alloc_rx_buffers(struct igbvf_ring *rx_ring,
* such as IA-64).
*/
wmb();
- writel(i, adapter->hw.hw_addr + rx_ring->tail);
+ writel_relaxed(i, adapter->hw.hw_addr + rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
}
@@ -2298,7 +2303,7 @@ static inline void igbvf_tx_queue_adv(struct igbvf_adapter *adapter,
tx_ring->buffer_info[first].next_to_watch = tx_desc;
tx_ring->next_to_use = i;
- writel(i, adapter->hw.hw_addr + tx_ring->tail);
+ writel_relaxed(i, adapter->hw.hw_addr + tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
*/
--
2.7.4
^ permalink raw reply related
* [PATCH v6 4/7] igb: eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
Sinan Kaya, intel-wired-lan, linux-kernel
In-Reply-To: <1521829277-9398-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a barrier
on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing the
register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/intel/igb/igb_main.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 3d4ff3c..570af25 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -5674,7 +5674,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
igb_maybe_stop_tx(tx_ring, DESC_NEEDED);
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- writel(i, tx_ring->tail);
+ writel_relaxed(i, tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
@@ -8079,7 +8079,12 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
* such as IA-64).
*/
wmb();
- writel(i, rx_ring->tail);
+ writel_relaxed(i, rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH v6 5/7] fm10k: Eliminate duplicate barriers on weakly-ordered archs
From: Sinan Kaya @ 2018-03-23 18:21 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, timur, sulrich, linux-arm-msm, linux-arm-kernel,
Sinan Kaya, intel-wired-lan, linux-kernel
In-Reply-To: <1521829277-9398-1-git-send-email-okaya@codeaurora.org>
Code includes wmb() followed by writel(). writel() already has a
barrier on some architectures like arm64.
This ends up CPU observing two barriers back to back before executing
the register write.
Since code already has an explicit barrier call, changing writel() to
writel_relaxed().
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 409554d..360ff9b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -180,7 +180,12 @@ void fm10k_alloc_rx_buffers(struct fm10k_ring *rx_ring, u16 cleaned_count)
wmb();
/* notify hardware of new descriptors */
- writel(i, rx_ring->tail);
+ writel_relaxed(i, rx_ring->tail);
+
+ /* We need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
}
@@ -1055,7 +1060,7 @@ static void fm10k_tx_map(struct fm10k_ring *tx_ring,
/* notify HW of packet */
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- writel(i, tx_ring->tail);
+ writel_relaxed(i, tx_ring->tail);
/* we need this if more than one processor can write to our tail
* at a time, it synchronizes IO on IA64/Altix systems
--
2.7.4
^ 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