On 01/06/2018 05:24 PM, Stephen Hemminger wrote:
On Sat, 6 Jan 2018 13:49:50 +0300
Andrew Rybchenko <arybche...@solarflare.com> wrote:
On 01/06/2018 04:06 AM, Stephen Hemminger wrote:
+}
+
+void
+_rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
+ struct rte_eth_link *link)
+{
+ const uint64_t *src = (const uint64_t *)&(dev->data->dev_link);
+ volatile uint64_t *dst = (uint64_t *)link;
+
+ RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
+
+ /*
+ * Note: this should never fail since both destination and expected
+ * values are the same and are a pointer from caller.
+ */
+ rte_atomic64_cmpset(dst, *dst, *src);
As I understand nobody says that *link (i.e. *dst) is initialized, so it
could be
reading uninitialized value. Not fatal, but not good as well.
May be it is better to use something like rte_atomic64_read().
link is a pointer passed in by the caller.
If it is uninitialized, that is still ok since it will have some value.
I thought about valgrind. Yes, it is problematic to run with hugepages,
but should be possible (with some patches). If so, it will/can complain
about usage of uninitialized value.
The different question is that since is almost backwards use of cmpset,
not sure if processor really guarantees atomic read of source pointer.
But this code is cloned from original in Intel driver so it is bug
for bug compatiable!
I see, but if so it was local to corresponding Intel driver. Now it is
becoming global.
