From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id A42281B16A for ; Sat, 6 Jan 2018 15:35:19 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 5FCAE100071; Sat, 6 Jan 2018 14:35:17 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Sat, 6 Jan 2018 14:35:11 +0000 To: Stephen Hemminger CC: References: <20180106010656.9167-1-stephen@networkplumber.org> <20180106010656.9167-3-stephen@networkplumber.org> <20180106062456.1d1eba50@xeon-e3> From: Andrew Rybchenko Message-ID: <750b9eff-9ba0-60b3-e587-fb22c2bfc5b7@solarflare.com> Date: Sat, 6 Jan 2018 17:35:06 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180106062456.1d1eba50@xeon-e3> Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23576.003 X-TM-AS-Result: No--10.653100-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1515249318-PC8dgd90ofxg Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2 02/15] ethdev: add linkstatus get/set helper functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Jan 2018 14:35:20 -0000 On 01/06/2018 05:24 PM, Stephen Hemminger wrote: > On Sat, 6 Jan 2018 13:49:50 +0300 > Andrew Rybchenko 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.