* Re: [Intel-wired-lan] [next-queue 09/10] ixgbe: ipsec offload stats
From: Alexander Duyck @ 2017-12-05 19:53 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <1512452116-14795-10-git-send-email-shannon.nelson@oracle.com>
On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> Add a simple statistic to count the ipsec offloads.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 28 ++++++++++++++----------
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 3 +++
> 3 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index 68097fe..bb66c85 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -265,6 +265,7 @@ struct ixgbe_rx_buffer {
> struct ixgbe_queue_stats {
> u64 packets;
> u64 bytes;
> + u64 ipsec_offloads;
> };
>
> struct ixgbe_tx_queue_stats {
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index c3e7a81..dddbc74 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -1233,34 +1233,34 @@ static void ixgbe_get_ethtool_stats(struct net_device *netdev,
> for (j = 0; j < netdev->num_tx_queues; j++) {
> ring = adapter->tx_ring[j];
> if (!ring) {
> - data[i] = 0;
> - data[i+1] = 0;
> - i += 2;
> + data[i++] = 0;
> + data[i++] = 0;
> + data[i++] = 0;
> continue;
> }
>
> do {
> start = u64_stats_fetch_begin_irq(&ring->syncp);
> - data[i] = ring->stats.packets;
> - data[i+1] = ring->stats.bytes;
> + data[i++] = ring->stats.packets;
> + data[i++] = ring->stats.bytes;
> + data[i++] = ring->stats.ipsec_offloads;
> } while (u64_stats_fetch_retry_irq(&ring->syncp, start));
> - i += 2;
> }
> for (j = 0; j < IXGBE_NUM_RX_QUEUES; j++) {
> ring = adapter->rx_ring[j];
> if (!ring) {
> - data[i] = 0;
> - data[i+1] = 0;
> - i += 2;
> + data[i++] = 0;
> + data[i++] = 0;
> + data[i++] = 0;
> continue;
> }
>
> do {
> start = u64_stats_fetch_begin_irq(&ring->syncp);
> - data[i] = ring->stats.packets;
> - data[i+1] = ring->stats.bytes;
> + data[i++] = ring->stats.packets;
> + data[i++] = ring->stats.bytes;
> + data[i++] = ring->stats.ipsec_offloads;
> } while (u64_stats_fetch_retry_irq(&ring->syncp, start));
> - i += 2;
> }
>
> for (j = 0; j < IXGBE_MAX_PACKET_BUFFERS; j++) {
> @@ -1297,12 +1297,16 @@ static void ixgbe_get_strings(struct net_device *netdev, u32 stringset,
> p += ETH_GSTRING_LEN;
> sprintf(p, "tx_queue_%u_bytes", i);
> p += ETH_GSTRING_LEN;
> + sprintf(p, "tx_queue_%u_ipsec_offloads", i);
> + p += ETH_GSTRING_LEN;
> }
> for (i = 0; i < IXGBE_NUM_RX_QUEUES; i++) {
> sprintf(p, "rx_queue_%u_packets", i);
> p += ETH_GSTRING_LEN;
> sprintf(p, "rx_queue_%u_bytes", i);
> p += ETH_GSTRING_LEN;
> + sprintf(p, "rx_queue_%u_ipsec_offloads", i);
> + p += ETH_GSTRING_LEN;
> }
> for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++) {
> sprintf(p, "tx_pb_%u_pxon", i);
I probably wouldn't bother reporting this per ring. It might make more
sense to handle this as an adapter statistic.
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 2a0dd7a..d1220bf 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -782,6 +782,7 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring, struct sk_buff *skb,
> if (tsa->encrypt)
> itd->flags |= IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN;
>
> + tx_ring->stats.ipsec_offloads++;
> return 1;
Instead of doing this here you may want to make it a part of the Tx
clean-up path. You should still have the flag bit set so you could
test a test for the IPSEC flag bit and if it is set on the tx_buffer
following the transmit you could then increment it there.
> }
>
> @@ -843,6 +844,8 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
> xo = xfrm_offload(skb);
> xo->flags = CRYPTO_DONE;
> xo->status = CRYPTO_SUCCESS;
> +
> + rx_ring->stats.ipsec_offloads++;
> }
>
> /**
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [Patch net v2] tipc: fix a null pointer deref on error path
From: David Miller @ 2017-12-05 19:53 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, tipc-discussion, jon.maloy, ying.xue
In-Reply-To: <20171204183143.7395-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 4 Dec 2017 10:31:43 -0800
> In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails
> we call tipc_close_conn() to clean up, but in this case
> calling conn_put() is just enough.
>
> This fixes the folllowing crash:
...
> Fixes: 14c04493cb77 ("tipc: add ability to order and receive topology events in driver")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [Patch net-next v2] net_sched: get rid of rcu_barrier() in tcf_block_put_ext()
From: David Miller @ 2017-12-05 19:53 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, pabeni, edumazet, jiri, jhs
In-Reply-To: <20171204184818.23960-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 4 Dec 2017 10:48:18 -0800
> Both Eric and Paolo noticed the rcu_barrier() we use in
> tcf_block_put_ext() could be a performance bottleneck when
> we have a lot of tc classes.
>
> Paolo provided the following to demonstrate the issue:
...
> Tested-by: Paolo Abeni <pabeni@redhat.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied.
^ permalink raw reply
* Re: [net 1/1] tipc: fix memory leak in tipc_accept_from_sock()
From: David Miller @ 2017-12-05 19:54 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen,
hoang.h.le, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1512421220-16773-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Mon, 4 Dec 2017 22:00:20 +0100
> When the function tipc_accept_from_sock() fails to create an instance of
> struct tipc_subscriber it omits to free the already created instance of
> struct tipc_conn instance before it returns.
>
> We fix that with this commit.
>
> Reported-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
From: David Miller @ 2017-12-05 19:55 UTC (permalink / raw)
To: simo.ghannam; +Cc: netdev
In-Reply-To: <5a25b7e1.08a5df0a.997e2.12bf@mx.google.com>
From: simo.ghannam@gmail.com
Date: Mon, 4 Dec 2017 22:02:06 +0100
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> Whenever the sock object is in DCCP_CLOSED state, dccp_disconnect() must free
> dccps_hc_tx_ccid and dccps_hc_rx_ccid and set to NULL.
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
Please eliminate this weird indentation in your commit messages.
The text should be adjustest fully to the leftmost column with
no unnecessary whitepsace.
Thank you.
^ permalink raw reply
* [PATCH net] netlink: Relax attr validation for fixed length types
From: David Ahern @ 2017-12-05 19:55 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require
exact length for some types") requires attributes using types NLA_U* and
NLA_S* to have an exact length. This change is exposing bugs in various
userspace commands that are sending attributes with an invalid length
(e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While
the commands are clearly broken and need to be fixed, users are arguing
that the sudden change in enforcement is breaking older commands on
newer kernels for use cases that otherwise "worked".
Relax the validation to print a warning mesage similar to what is done
for messages containing extra bytes after parsing.
Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
lib/nlattr.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 8bf78b4b78f0..6122662906c8 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -28,8 +28,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
};
static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
+ [NLA_U8] = sizeof(u8),
+ [NLA_U16] = sizeof(u16),
+ [NLA_U32] = sizeof(u32),
+ [NLA_U64] = sizeof(u64),
[NLA_MSECS] = sizeof(u64),
[NLA_NESTED] = NLA_HDRLEN,
+ [NLA_S8] = sizeof(s8),
+ [NLA_S16] = sizeof(s16),
+ [NLA_S32] = sizeof(s32),
+ [NLA_S64] = sizeof(s64),
};
static int validate_nla_bitfield32(const struct nlattr *nla,
@@ -70,10 +78,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
BUG_ON(pt->type > NLA_TYPE_MAX);
/* for data types NLA_U* and NLA_S* require exact length */
- if (nla_attr_len[pt->type]) {
- if (attrlen != nla_attr_len[pt->type])
- return -ERANGE;
- return 0;
+ if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
+ pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
+ current->comm, type);
}
switch (pt->type) {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Peter Zijlstra @ 2017-12-05 19:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: tglx, kvm, fweisbec, jiangshanlai, linux-kernel, rostedt, josh,
dhowells, edumazet, netdev, mathieu.desnoyers, oleg, dipankar,
akpm, Paul E. McKenney, virtualization, mingo
In-Reply-To: <20171205212053-mutt-send-email-mst@kernel.org>
On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 08:17:33PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 08:57:46PM +0200, Michael S. Tsirkin wrote:
> >
> > > I don't see WRITE_ONCE inserting any barriers, release or
> > > write.
> >
> > Correct, never claimed there was.
> >
> > Just saying that:
> >
> > obj = READ_ONCE(*foo);
> > val = READ_ONCE(obj->val);
> >
> > Never needs a barrier (except on Alpha and we want to make that go
> > away). Simply because a CPU needs to complete the load of @obj before it
> > can compute the address &obj->val. Thus the second load _must_ come
> > after the first load and we get LOAD-LOAD ordering.
> >
> > Alpha messing that up is a royal pain, and Alpha not being an
> > active/living architecture is just not worth the pain of keeping this in
> > the generic model.
> >
>
> Right. What I am saying is that for writes you need
>
> WRITE_ONCE(obj->val, 1);
> smp_wmb();
> WRITE_ONCE(*foo, obj);
You really should use smp_store_release() here instead of the smp_wmb().
But yes.
> and this barrier is no longer paired with anything until
> you realize there's a dependency barrier within READ_ONCE.
No, there isn't. read_dependecy barriers are no more. They don't exist.
They never did, except on Alpha anyway.
There were a ton of sites that relied on this but never had the
smp_read_barrier_depends() annotation, similarly there were quite a few
sites that had the barrier but nobody could explain wtf for.
What these patches do is return the sane rule that dependent loads are
ordered.
And like all memory ordering; it should come with comments. Any piece of
code that relies on memory ordering and doesn't have big fat comments
that explain the required ordering are broken per definition. Maybe not
now, but they will be after a few years because someone changed it and
didn't know.
> Barrier pairing was a useful tool to check code validity,
> maybe there are other, better tools now.
Same is true for the closely related control dependency, you can pair
against those just fine but they don't have an explicit barrier
annotation.
There are no tools, use brain.
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: dsa: use per-port upstream port
From: Vivien Didelot @ 2017-12-05 19:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20171205.144727.522098543209254657.davem@davemloft.net>
Hi David,
David Miller <davem@davemloft.net> writes:
> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Mon, 4 Dec 2017 12:34:52 -0500
>
>> An upstream port is a local switch port used to reach a CPU port.
>>
>> DSA still considers a unique CPU port in the whole switch fabric and
>> thus return a unique upstream port for a given switch. This is wrong in
>> a multiple CPU ports environment.
>>
>> We are now switching to using the dedicated CPU port assigned to each
>> port in order to get rid of the deprecated unique tree CPU port.
>>
>> This patchset makes the dsa_upstream_port() helper take a port argument
>> and goes one step closer complete support for multiple CPU ports.
>
> Please adhere to reverse-christmas-tree for variable declarations in
> these changes.
Totally makes sense for this month of the year, I'm replanting a v2!
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Peter Zijlstra @ 2017-12-05 19:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: tglx, kvm, fweisbec, jiangshanlai, linux-kernel, rostedt, josh,
dhowells, edumazet, netdev, mathieu.desnoyers, oleg, dipankar,
akpm, Paul E. McKenney, virtualization, mingo
In-Reply-To: <20171205215020-mutt-send-email-mst@kernel.org>
On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > WRITE_ONCE(obj->val, 1);
> > > smp_wmb();
> > > WRITE_ONCE(*foo, obj);
> >
> > I believe Peter was instead suggesting:
> >
> > WRITE_ONCE(obj->val, 1);
> > smp_store_release(foo, obj);
>
> Isn't that more expensive though?
Depends on the architecture. The only architecture where it is more
expensive and people actually still care about is ARM I think.
^ permalink raw reply
* Re: [PATCH net-next 00/10] nfp: enhanced debug dump via ethtool
From: David Miller @ 2017-12-05 20:01 UTC (permalink / raw)
To: simon.horman; +Cc: jakub.kicinski, netdev, oss-drivers, carl.heymann
In-Reply-To: <20171204223421.19174-1-simon.horman@netronome.com>
From: Simon Horman <simon.horman@netronome.com>
Date: Mon, 4 Dec 2017 23:34:11 +0100
> Add debug dump implementation to the NFP driver. This makes use of
> existing ethtool infrastructure. ethtool -W is used to select the dump
> level and ethtool -w is used to dump NFP state.
>
> The existing behaviour of dump level 0, dumping the arm.diag resource, is
> preserved. Dump levels greater than 0 are implemented by this patchset and
> optionally supported by firmware providing a _abi_dump_spec rtsym. This
> rtsym provides a specification, in TLV format, of the information to be
> dumped from the NFP at each supported dump level.
>
> Dumps are also structured using a TLVs. They consist a prolog and the data
> described int he corresponding dump.
Series applied, thanks Simon.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: sched: sch_api: fix coding style issues for extack
From: David Miller @ 2017-12-05 20:04 UTC (permalink / raw)
To: aring; +Cc: jhs, xiyou.wangcong, jiri, netdev, dsahern
In-Reply-To: <20171204234000.20756-1-aring@mojatatu.com>
From: Alexander Aring <aring@mojatatu.com>
Date: Mon, 4 Dec 2017 18:39:58 -0500
> this patch prepares to handle extack for qdiscs and fixes checkpatch
> issues.
>
> There are a bunch of warnings issued by checkpatch which bothered me.
> This first patchset is to get rid of those warnings to make way for
> the next patchsets.
>
> I plan to followup with qdiscs, classifiers and actions after this.
Series applied.
^ permalink raw reply
* Re: Linux ECN Handling
From: Neal Cardwell @ 2017-12-05 20:04 UTC (permalink / raw)
To: Steve Ibanez
Cc: Eric Dumazet, Yuchung Cheng, Daniel Borkmann, Netdev,
Florian Westphal, Mohammad Alizadeh, Lawrence Brakmo
In-Reply-To: <CACJspmLaxdHoa63jCuD-mKJS35BZ69b9qw3tEZjFxbUNb3PSHg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]
On Tue, Dec 5, 2017 at 2:36 PM, Steve Ibanez <sibanez@stanford.edu> wrote:
> Hi Neal,
>
> I've included a link to small trace of 13 packets which is different
> from the screenshot I attached in my last email, but shows the same
> sequence of events. It's a bit hard to read the tcptrace due to the
> 300ms timeout, so I figured this was the best approach.
>
> slice.pcap: https://drive.google.com/open?id=1hYXbUClHGbQv1hWG1HZWDO2WYf30N6G8
Thanks for the trace! Attached is a screen shot (first screen shot is
for the arriving packets with CWR; second is after the RTO). The
sender behavior looks reasonable. I don't see why the receiver is not
ACKing. As you say, it does look like a receiver bug. You could try
adding instrumentation to try to isolate why the receiver is not
sending an ACK immediately. You might instrument __tcp_ack_snd_check()
and tcp_send_delayed_ack() so that when the most recent incoming
packet had cwr set they printk to log what they are deciding in this
case. Perhaps the tcp_send_delayed_ack() code is hitting the max_ato
= HZ / 2 code path?
neal
[-- Attachment #2: cwr-no-ack-1.png --]
[-- Type: image/png, Size: 9725 bytes --]
[-- Attachment #3: cwr-no-ack-2.png --]
[-- Type: image/png, Size: 11703 bytes --]
^ permalink raw reply
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
From: Eric Dumazet @ 2017-12-05 20:07 UTC (permalink / raw)
To: Craig Gallek; +Cc: Eric Dumazet, Alexander Potapenko, David Miller, Networking
In-Reply-To: <CAEfhGiwQe-whM8p1OuLC3WntogmC6AeLS0HPRUcTL85ii8hKmw@mail.gmail.com>
On Tue, 2017-12-05 at 14:39 -0500, Craig Gallek wrote:
> On Tue, Dec 5, 2017 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com>
> wrote:
> > On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
> > >
> > > + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
> >
> > Typo here, this needs sk_nulls_node of course.
> >
>
> Thanks Eric, this looks good to me. The tail insertion is still
> required in udp_lib_get_port for the second layer hash, but not here.
> fwiw, reuseport_dualstack in the selftests directory verifies this
> behavior. I tried it with your patch (it still passes) and removing
> the udp_lib_get_port path (to make sure it breaks when it should).
>
Thanks for confirming this, I will send the official patch then ;)
^ permalink raw reply
* Re: [PATCH] VSOCK: fix outdated sk_state value in hvs_release()
From: David Miller @ 2017-12-05 20:07 UTC (permalink / raw)
To: stefanha; +Cc: netdev, jhansen, decui, cavery
In-Reply-To: <20171205113114.3377-1-stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 5 Dec 2017 11:31:14 +0000
> Since commit 3b4477d2dcf2709d0be89e2a8dced3d0f4a017f2 ("VSOCK: use TCP
> state constants for sk_state") VSOCK has used TCP_* constants for
> sk_state.
>
> Commit b4562ca7925a3bedada87a3dd072dd5bad043288 ("hv_sock: add locking
> in the open/close/release code paths") reintroduced the SS_DISCONNECTING
> constant.
>
> This patch replaces the old SS_DISCONNECTING with the new TCP_CLOSING
> constant.
>
> CC: Dexuan Cui <decui@microsoft.com>
> CC: Cathy Avery <cavery@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Paul E. McKenney @ 2017-12-05 20:08 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
fweisbec, oleg, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20171205215020-mutt-send-email-mst@kernel.org>
On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 11:33:39AM -0800, Paul E. McKenney wrote:
> > On Tue, Dec 05, 2017 at 09:24:21PM +0200, Michael S. Tsirkin wrote:
[ . . . ]
> > > and this barrier is no longer paired with anything until
> > > you realize there's a dependency barrier within READ_ONCE.
> > >
> > > Barrier pairing was a useful tool to check code validity,
> > > maybe there are other, better tools now.
> >
> > There are quite a few people who say that smp_store_release() is
> > easier for the tools to analyze than is smp_wmb(). My experience with
> > smp_read_barrier_depends() and rcu_dereference() leads me to believe
> > that they are correct.
>
> OK, but smp_store_release is still not paired with anything since we
> rely on READ_ONCE to include the implicit dpendendency barrier.
Why wouldn't you consider the smp_store_release() to be paired with
the new improved READ_ONCE()?
Thanx, Paul
^ permalink raw reply
* Re: Uninitialized value in __sk_nulls_add_node_rcu()
From: Craig Gallek @ 2017-12-05 20:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, Alexander Potapenko, David Miller, Networking
On Tue, Dec 5, 2017 at 3:07 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2017-12-05 at 14:39 -0500, Craig Gallek wrote:
>> On Tue, Dec 5, 2017 at 9:18 AM, Eric Dumazet <eric.dumazet@gmail.com>
>> wrote:
>> > On Tue, 2017-12-05 at 06:15 -0800, Eric Dumazet wrote:
>> > >
>> > > + hlist_nulls_add_head_rcu(&sk->sk_nulss_node, list);
>> >
>> > Typo here, this needs sk_nulls_node of course.
>> >
>>
>> Thanks Eric, this looks good to me. The tail insertion is still
>> required in udp_lib_get_port for the second layer hash, but not here.
>> fwiw, reuseport_dualstack in the selftests directory verifies this
>> behavior. I tried it with your patch (it still passes) and removing
>> the udp_lib_get_port path (to make sure it breaks when it should).
>>
>
> Thanks for confirming this, I will send the official patch then ;)
Great, feel free to include my Acked-by.
^ permalink raw reply
* Re: [PATCH v3 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint
From: David Miller @ 2017-12-05 20:11 UTC (permalink / raw)
To: laoar.shao
Cc: songliubraving, marcelo.leitner, kuznet, yoshfuji, rostedt,
bgregg, netdev, linux-kernel
In-Reply-To: <1512483162-5557-1-git-send-email-laoar.shao@gmail.com>
From: Yafang Shao <laoar.shao@gmail.com>
Date: Tue, 5 Dec 2017 14:12:42 +0000
> }
>
> +/* For tcp_set_state tracepoint */
> +void sk_state_store(struct sock *sk, int newstate);
> +
> void sock_enable_timestamp(struct sock *sk, int flag);
> int sock_get_timestamp(struct sock *, struct timeval __user *);
> int sock_get_timestampns(struct sock *, struct timespec __user *);
...
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2036,6 +2036,18 @@ int tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int nonblock,
> }
> EXPORT_SYMBOL(tcp_recvmsg);
>
> +void sk_state_store(struct sock *sk, int newstate)
> +{
> + trace_tcp_set_state(sk, sk->sk_state, newstate);
> + __sk_state_store(sk, newstate);
> +}
> +
Please do not define a sock generic function in the TCP protocol code.
If it belongs in TCP and is only used by TCP, give is a tcp specific
name prefix rather than a generic sk_ one.
This is even more ugly because I see that inet_connection_sock.c and
inet_hashtables.c uses this thing too. Which means that DCCP sockets
will trigger this tracepoint.
inet_connection_sock.c and inet_hashtables.c are not guaranteed to
operate on only TCP sockets, and you must make amends to handle that
properly.
Thanks.
^ permalink raw reply
* Re: [Intel-wired-lan] [next-queue 10/10] ixgbe: register ipsec offload with the xfrm subsystem
From: Alexander Duyck @ 2017-12-05 20:11 UTC (permalink / raw)
To: Shannon Nelson
Cc: intel-wired-lan, Jeff Kirsher, Steffen Klassert, Sowmini Varadhan,
Netdev
In-Reply-To: <1512452116-14795-11-git-send-email-shannon.nelson@oracle.com>
On Mon, Dec 4, 2017 at 9:35 PM, Shannon Nelson
<shannon.nelson@oracle.com> wrote:
> With all the support code in place we can now link in the ipsec
> offload operations and set the ESP feature flag for the XFRM
> subsystem to see.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 4 ++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index d1220bf..0d5497b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -884,6 +884,10 @@ void ixgbe_init_ipsec_offload(struct ixgbe_adapter *adapter)
> ixgbe_ipsec_clear_hw_tables(adapter);
> ixgbe_ipsec_stop_engine(adapter);
>
> + adapter->netdev->xfrmdev_ops = &ixgbe_xfrmdev_ops;
> + adapter->netdev->features |= NETIF_F_HW_ESP;
> + adapter->netdev->hw_enc_features |= NETIF_F_HW_ESP;
> +
> return;
> err:
> if (ipsec) {
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index c857594..9231351 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -9799,6 +9799,10 @@ ixgbe_features_check(struct sk_buff *skb, struct net_device *dev,
> if (skb->encapsulation && !(features & NETIF_F_TSO_MANGLEID))
> features &= ~NETIF_F_TSO;
>
> + /* IPsec offload doesn't get along well with others *yet* */
> + if (skb->sp)
> + features &= ~(NETIF_F_TSO | NETIF_F_HW_CSUM_BIT);
I'm pretty sure the feature flag stripping here isn't correct. The
feature bits you want to strip would probably be consistent with the
network_hdr_len check bits included before the MANGLEID check.
We should do some digging into this as it may be a kernel issue. I'm
just wondering if ipsec updates any headers such as the transport
offset or skb checksum start. If either of those are updated that
would explain the issues with getting the offloads to work.
> +
> return features;
> }
>
> --
> 2.7.4
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: [PATCH 1/2] net: sh_eth: add support for SH7786
From: David Miller @ 2017-12-05 20:14 UTC (permalink / raw)
To: sergei.shtylyov
Cc: thomas.petazzoni, niklas.soderlund+renesas, geert+renesas,
horms+renesas, netdev, linux-renesas-soc
In-Reply-To: <a35c703f-9546-d4b7-25c4-681726bcea92@cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Tue, 5 Dec 2017 22:49:10 +0300
> On 12/05/2017 10:04 PM, Sergei Shtylyov wrote:
>
>>>>> This commit adds the sh_eth_cpu_data structure that describes the
>>>>> SH7786 variant of the IP.
>>>>
>>>> The manual seems to be unavailable, so I have to trust you. :-)
>>>
>>> Yes, sadly. However, if you tell me what to double check, I'd be happy
>>> to do so.
>> I have the manual now, will check against it...
>> DaveM, I'm retracting my ACK for the time being.
>
> Starting to look into the manual, the current patch is wrong. SH7786
> SoC was probably the 1st one to use what we thought was R-Car specific
> register layout. Definite NAK on this version.
Ok.
^ permalink raw reply
* [PATCH 1/2] net: macb: reduce scope of rx_fs_lock-protected regions
From: Julia Cartwright @ 2017-12-05 20:17 UTC (permalink / raw)
To: Julia Lawall
Cc: Rafal Ozieblo, Nicolas Ferre, netdev, linux-kernel, kbuild-all
In-Reply-To: <alpine.DEB.2.20.1712020800010.2168@hadrien>
Commit ae8223de3df5 ("net: macb: Added support for RX filtering")
introduces a lock, rx_fs_lock which is intended to protect the list of
rx_flow items and synchronize access to the hardware rx filtering
registers.
However, the region protected by this lock is overscoped, unnecessarily
including things like slab allocation. Reduce this lock scope to only
include operations which must be performed atomically: list traversal,
addition, and removal, and hitting the macb filtering registers.
This fixes the use of kmalloc w/ GFP_KERNEL in atomic context.
Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering")
Cc: Rafal Ozieblo <rafalo@cadence.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
While Julia Lawall's cocci-generated patch fixes the problem, the right
solution is to obviate the problem altogether.
Thanks,
The Other Julia
drivers/net/ethernet/cadence/macb_main.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c5fa87cdc6c4..e7ef104a077d 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2796,6 +2796,7 @@ static int gem_add_flow_filter(struct net_device *netdev,
struct macb *bp = netdev_priv(netdev);
struct ethtool_rx_flow_spec *fs = &cmd->fs;
struct ethtool_rx_fs_item *item, *newfs;
+ unsigned long flags;
int ret = -EINVAL;
bool added = false;
@@ -2811,6 +2812,8 @@ static int gem_add_flow_filter(struct net_device *netdev,
htonl(fs->h_u.tcp_ip4_spec.ip4dst),
htons(fs->h_u.tcp_ip4_spec.psrc), htons(fs->h_u.tcp_ip4_spec.pdst));
+ spin_lock_irqsave(&bp->rx_fs_lock, flags);
+
/* find correct place to add in list */
if (list_empty(&bp->rx_fs_list.list))
list_add(&newfs->list, &bp->rx_fs_list.list);
@@ -2837,9 +2840,11 @@ static int gem_add_flow_filter(struct net_device *netdev,
if (netdev->features & NETIF_F_NTUPLE)
gem_enable_flow_filters(bp, 1);
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return 0;
err:
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
kfree(newfs);
return ret;
}
@@ -2850,9 +2855,14 @@ static int gem_del_flow_filter(struct net_device *netdev,
struct macb *bp = netdev_priv(netdev);
struct ethtool_rx_fs_item *item;
struct ethtool_rx_flow_spec *fs;
+ unsigned long flags;
- if (list_empty(&bp->rx_fs_list.list))
+ spin_lock_irqsave(&bp->rx_fs_lock, flags);
+
+ if (list_empty(&bp->rx_fs_list.list)) {
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return -EINVAL;
+ }
list_for_each_entry(item, &bp->rx_fs_list.list, list) {
if (item->fs.location == cmd->fs.location) {
@@ -2869,12 +2879,14 @@ static int gem_del_flow_filter(struct net_device *netdev,
gem_writel_n(bp, SCRT2, fs->location, 0);
list_del(&item->list);
- kfree(item);
bp->rx_fs_list.count--;
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
+ kfree(item);
return 0;
}
}
+ spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return -EINVAL;
}
@@ -2943,11 +2955,8 @@ static int gem_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
{
struct macb *bp = netdev_priv(netdev);
- unsigned long flags;
int ret;
- spin_lock_irqsave(&bp->rx_fs_lock, flags);
-
switch (cmd->cmd) {
case ETHTOOL_SRXCLSRLINS:
if ((cmd->fs.location >= bp->max_tuples)
@@ -2966,7 +2975,6 @@ static int gem_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
ret = -EOPNOTSUPP;
}
- spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
return ret;
}
--
2.14.2
^ permalink raw reply related
* [PATCH 2/2] net: macb: kill useless use of list_empty()
From: Julia Cartwright @ 2017-12-05 20:18 UTC (permalink / raw)
To: Julia Lawall
Cc: Rafal Ozieblo, Nicolas Ferre, netdev, linux-kernel, kbuild-all
In-Reply-To: <20171205201711.GA18445@jcartwri.amer.corp.natinst.com>
The list_for_each_entry() macro already handles the case where the list
is empty (by not executing the loop body). It's not necessary to handle
this case specially, so stop doing so.
Cc: Rafal Ozieblo <rafalo@cadence.com>
Signed-off-by: Julia Cartwright <julia@ni.com>
---
This is an additional cleanup patch found when looking at this code.
Julia
drivers/net/ethernet/cadence/macb_main.c | 34 ++++++++++++--------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index e7ef104a077d..3643c6ad2322 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2815,25 +2815,22 @@ static int gem_add_flow_filter(struct net_device *netdev,
spin_lock_irqsave(&bp->rx_fs_lock, flags);
/* find correct place to add in list */
- if (list_empty(&bp->rx_fs_list.list))
- list_add(&newfs->list, &bp->rx_fs_list.list);
- else {
- list_for_each_entry(item, &bp->rx_fs_list.list, list) {
- if (item->fs.location > newfs->fs.location) {
- list_add_tail(&newfs->list, &item->list);
- added = true;
- break;
- } else if (item->fs.location == fs->location) {
- netdev_err(netdev, "Rule not added: location %d not free!\n",
- fs->location);
- ret = -EBUSY;
- goto err;
- }
+ list_for_each_entry(item, &bp->rx_fs_list.list, list) {
+ if (item->fs.location > newfs->fs.location) {
+ list_add_tail(&newfs->list, &item->list);
+ added = true;
+ break;
+ } else if (item->fs.location == fs->location) {
+ netdev_err(netdev, "Rule not added: location %d not free!\n",
+ fs->location);
+ ret = -EBUSY;
+ goto err;
}
- if (!added)
- list_add_tail(&newfs->list, &bp->rx_fs_list.list);
}
+ if (!added)
+ list_add_tail(&newfs->list, &bp->rx_fs_list.list);
+
gem_prog_cmp_regs(bp, fs);
bp->rx_fs_list.count++;
/* enable filtering if NTUPLE on */
@@ -2859,11 +2856,6 @@ static int gem_del_flow_filter(struct net_device *netdev,
spin_lock_irqsave(&bp->rx_fs_lock, flags);
- if (list_empty(&bp->rx_fs_list.list)) {
- spin_unlock_irqrestore(&bp->rx_fs_lock, flags);
- return -EINVAL;
- }
-
list_for_each_entry(item, &bp->rx_fs_list.list, list) {
if (item->fs.location == cmd->fs.location) {
/* disable screener regs for the flow entry */
--
2.14.2
^ permalink raw reply related
* Re: [PATCH V11 3/5] printk: hash addresses printed with %p
From: Geert Uytterhoeven @ 2017-12-05 20:20 UTC (permalink / raw)
To: Tobin C. Harding
Cc: kernel-hardening, Linus Torvalds, Jason A. Donenfeld,
Theodore Ts'o, Kees Cook, Paolo Bonzini, Tycho Andersen,
Roberts, William C, Tejun Heo, Jordan Glover, Greg KH,
Petr Mladek, Joe Perches, Ian Campbell, Sergey Senozhatsky,
Catalin Marinas, Will Deacon, Steven Rostedt, Chris Fries,
Dave Weinstein
In-Reply-To: <1511921105-3647-4-git-send-email-me@tobin.cc>
Hi Tobin,
On Wed, Nov 29, 2017 at 3:05 AM, Tobin C. Harding <me@tobin.cc> wrote:
> Currently there exist approximately 14 000 places in the kernel where
> addresses are being printed using an unadorned %p. This potentially
> leaks sensitive information regarding the Kernel layout in memory. Many
> of these calls are stale, instead of fixing every call lets hash the
> address by default before printing. This will of course break some
> users, forcing code printing needed addresses to be updated.
>
> Code that _really_ needs the address will soon be able to use the new
> printk specifier %px to print the address.
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> +/* Maps a pointer to a 32 bit unique identifier. */
> +static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
> +{
> + unsigned long hashval;
> + const int default_width = 2 * sizeof(ptr);
> +
> + if (unlikely(!have_filled_random_ptr_key)) {
> + spec.field_width = default_width;
> + /* string length must be less than default_width */
> + return string(buf, end, "(ptrval)", spec);
> + }
> +
> +#ifdef CONFIG_64BIT
> + hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
> + /*
> + * Mask off the first 32 bits, this makes explicit that we have
> + * modified the address (and 32 bits is plenty for a unique ID).
> + */
> + hashval = hashval & 0xffffffff;
> +#else
> + hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
> +#endif
Would it make sense to keep the 3 lowest bits of the address?
Currently printed pointers no longer have any correlation with the actual
alignment in memory of the object, which is a typical cause of a class of bugs.
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH] ethtool: Add ETHTOOL_RESET support via --reset command
From: Scott Branden @ 2017-12-05 20:24 UTC (permalink / raw)
To: Gal Pressman, John W. Linville
Cc: BCM Kernel Feedback, Steve Lin, Michael Chan, netdev
In-Reply-To: <f9f26d96-5707-dcc9-58b1-c2636211c6a3@mellanox.com>
On 17-12-05 12:21 AM, Gal Pressman wrote:
> On 04-Dec-17 22:16, Scott Branden wrote:
>> Hi Gal,
>>
>> I do not understand you're comment - questions inline
>>
>>
>> On 17-12-03 12:07 AM, Gal Pressman wrote:
>>> On 30-Nov-17 21:24, Scott Branden wrote:
>>>> Add ETHTOOL_RESET support via --reset command.
>>>>
>>>> ie. ethtool --reset DEVNAME <flagname(s)>
>>>>
>>>> flagnames currently match the ETH_RESET_xxx names:
>>>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
>>>>
>>>> Alternatively, you can specific component bitfield directly using
>>>> ethtool --reset DEVNAME flags %x
>>>>
>>>> Signed-off-by: Scott Branden <scott.branden@broadcom.com>
>>>> ---
>>>> ethtool.8.in | 55 ++++++++++++++++++++++++++++++++++++++++++++-
>>>> ethtool.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 127 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/ethtool.8.in b/ethtool.8.in
>>>> index 6ad3065..925cfe3 100644
>>>> --- a/ethtool.8.in
>>>> +++ b/ethtool.8.in
>>>> @@ -355,6 +355,20 @@ ethtool \- query or control network driver and hardware settings
>>>> .B ethtool \-\-get\-phy\-tunable
>>>> .I devname
>>>> .RB [ downshift ]
>>>> +.HP
>>>> +.B ethtool \-\-reset
>>>> +.I devname
>>>> +.BN flags
>>>> +.B [mgmt]
>>>> +.B [irq]
>>>> +.B [dma]
>>>> +.B [filter]
>>>> +.B [offload]
>>>> +.B [mac]
>>>> +.B [phy]
>>>> +.B [ram]
>>>> +.B [dedicated]
>>>> +.B [all]
>>>> .
>>> Nit:
>>> Usually, the brackets formatting is different than the keyword inside them.
>> Could you please explain what the formatting should be then? Any of the options listed are possible components that can be selected. At least one must be chosen. Should the brackets be removed then?
> The brackets are fine, IMO .B [mgmt] should be replaced with .RB [ mgmt ].
> Looks better, and keeps us consistent with other flags (--get-phy-tunable for example).
OK - I'll update. Thanks.
>
>
^ permalink raw reply
* Re: ethtool-copy.h out of sync with linux header file?
From: Scott Branden @ 2017-12-05 20:25 UTC (permalink / raw)
To: Greenwalt, Paul, linville@tuxdriver.com; +Cc: netdev
In-Reply-To: <9A83C569D64F2541B0B591F2CBA3CC01CB2CE496@ORSMSX112.amr.corp.intel.com>
Hi Paul,
On 17-12-04 01:00 PM, Greenwalt, Paul wrote:
> John,
>
> Can this patch be reverted? As Stephen Hemminger mentioned
> there is an ABI compatibility issue with this patch:
I can add the revert into a patch series followed by the sync of
ethtool-copy.h with net-netx ethtool.h
>
> https://patchwork.ozlabs.org/patch/806049/#1757846
>
> Thanks,
> Paul
>
>> -----Original Message-----
>> From: Scott Branden [mailto:scott.branden@broadcom.com]
>> Sent: Monday, December 4, 2017 12:07 PM
>> To: linville@tuxdriver.com; Greenwalt, Paul <paul.greenwalt@intel.com>
>> Cc: netdev <netdev@vger.kernel.org>
>> Subject: ethtool-copy.h out of sync with linux header file?
>>
>> Hi Paul/John,
>>
>> I see a commit "ethtool: Add DMA Coalescing support" was added to
>> ethtool-copy.h yet I cannot find the change in the ethtool.h in the
>> linux kernel.
>>
>> As such, I am unable to commit a change to ethtool-copy.h to sync it
>> with net-next. If I sync then ethtool will fail to compile.
>>
>> Please advise.
>>
>> Thanks,
>>
>> Scott
>>
>>
^ permalink raw reply
* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Michael S. Tsirkin @ 2017-12-05 20:28 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Paul E. McKenney, linux-kernel, mingo, jiangshanlai, dipankar,
akpm, mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
fweisbec, oleg, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20171205195752.GS3165@worktop.lehotels.local>
On Tue, Dec 05, 2017 at 08:57:52PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 09:51:48PM +0200, Michael S. Tsirkin wrote:
> > > > WRITE_ONCE(obj->val, 1);
> > > > smp_wmb();
> > > > WRITE_ONCE(*foo, obj);
> > >
> > > I believe Peter was instead suggesting:
> > >
> > > WRITE_ONCE(obj->val, 1);
> > > smp_store_release(foo, obj);
> >
> > Isn't that more expensive though?
>
> Depends on the architecture. The only architecture where it is more
> expensive and people actually still care about is ARM I think.
Right. Why should I use the more expensive smp_store_release then?
--
MST
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox