From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BCE91A04C8; Fri, 18 Sep 2020 12:46:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8B3181D14D; Fri, 18 Sep 2020 12:46:51 +0200 (CEST) Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by dpdk.org (Postfix) with ESMTP id CFCFE1C1C3; Fri, 18 Sep 2020 12:46:49 +0200 (CEST) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 08IAiuxV193257; Fri, 18 Sep 2020 10:46:48 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=mime-version : message-id : date : from : sender : to : cc : subject : references : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=GrDMzvouktTeDEDN8TyY/M4Ot1d1CCzk56sczbyxibo=; b=qG2Whb/trE5tnzyI7NEsczZWhZCAzjYOod1rLLDvwn4dQqGWypL11uni6M/q0qohrZKJ D/nP2vXm00aYVmJNnxwjnrjRlpuXm1y/SEYJyBpYpToSyEjIzbJsfqZoZKbWknwrnRw0 TIJp+dlUoTRfRJlyTxsNFy30qHx9BlKkHKGfVg5EuP5sJ7I5dgXhvbaaBTh0hYOXL5SI GXCM8FPYE6Pb+jY11k7inDpBUKNkxdAR+Y85Q9GMv6dA89NhOlx5rcSX0h6mjTSE533p ObVxm4gk34iONCCsB+0UntvcHDVnuYjyQLUmyCpIuXAW/fr58dnQy2M1OqR7IARCECds Fw== Received: from aserp3030.oracle.com (aserp3030.oracle.com [141.146.126.71]) by aserp2120.oracle.com with ESMTP id 33gp9mpe83-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 18 Sep 2020 10:46:48 +0000 Received: from pps.filterd (aserp3030.oracle.com [127.0.0.1]) by aserp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 08IAk7jo157251; Fri, 18 Sep 2020 10:46:48 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserp3030.oracle.com with ESMTP id 33khppgw1a-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 18 Sep 2020 10:46:48 +0000 Received: from abhmp0005.oracle.com (abhmp0005.oracle.com [141.146.116.11]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id 08IAkkoh006933; Fri, 18 Sep 2020 10:46:46 GMT MIME-Version: 1.0 Message-ID: <1b510ced-ba3a-4bf9-8d33-204073cfedba@default> Date: Fri, 18 Sep 2020 10:46:44 +0000 (UTC) From: Vipul Ashri To: Edward Makarov , Andrew Rybchenko , dev@dpdk.org Cc: chenbo.xia@intel.com, zhihong.wang@intel.com, maxime.coquelin@redhat.com, stable@dpdk.org References: <20200814092138.1692-1-vipul.ashri@oracle.com> <7a2d66d6-3961-5251-36d2-c702fe80379f@solarflare.com> In-Reply-To: X-Priority: 3 X-Mailer: Oracle Beehive Extensions for Outlook 2.0.1.9.1 (1003210) [OL 15.0.5257.0 (x86)] Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9747 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=1 mlxlogscore=999 phishscore=0 mlxscore=0 adultscore=0 spamscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009180088 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9747 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999 adultscore=0 malwarescore=0 clxscore=1015 lowpriorityscore=0 phishscore=0 spamscore=0 priorityscore=1501 suspectscore=1 impostorscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009180088 Subject: Re: [dpdk-dev] [PATCH] net/virtio: fix wrong variable assignment in helper macro 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Edward / Andrew I like your suggestions and applied with [v5] net/virtio: fix wrong variabl= e assignment in helper macro V4 had a extra line typos., V5 is tested compiled and pushed carefully Than= ks! Regards Vipul -----Original Message----- From: Edward Makarov [mailto:makarov@kraftway.ru]=20 Sent: Sunday, 30 August, 2020 3:48 To: Andrew Rybchenko ; Vipul Ashri ; dev@dpdk.org Cc: chenbo.xia@intel.com; zhihong.wang@intel.com; maxime.coquelin@redhat.co= m; stable@dpdk.org Subject: Re: [dpdk-dev] [PATCH] net/virtio: fix wrong variable assignment i= n helper macro On 8/29/20 2:22 PM, Andrew Rybchenko wrote: > On 8/14/20 12:21 PM, Vipul Ashri wrote: >> Inside Macro ASSIGN_UNLESS_EQUAL(var, val), assignment to var is=20 >> always failing as assignment done using var_ having local scope only. >> This leads to TX packets not going out and found broken due to=20 >> cleanup malfunctioning. This patch fixes the wrong variable assignment. >> >> Fixes: 57f90f894588 ("net/virtio: reuse packed ring functions") >> Cc: stable@dpdk.org >> >> Signed-off-by: Vipul Ashri >> --- >> drivers/net/virtio/virtqueue.h | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/virtio/virtqueue.h=20 >> b/drivers/net/virtio/virtqueue.h index 105a9c00c..20c95471e 100644 >> --- a/drivers/net/virtio/virtqueue.h >> +++ b/drivers/net/virtio/virtqueue.h >> @@ -607,10 +607,8 @@ virtqueue_notify(struct virtqueue *vq) >> =20 >> /* avoid write operation when necessary, to lessen cache issues */ >> #define ASSIGN_UNLESS_EQUAL(var, val) do {=09\ >> -=09typeof(var) var_ =3D (var);=09=09\ >> -=09typeof(val) val_ =3D (val);=09=09\ >> -=09if ((var_) !=3D (val_))=09=09=09\ >> -=09=09(var_) =3D (val_);=09=09\ >> +=09if ((var) !=3D (val))=09=09=09\ >> +=09=09(var) =3D (val);=09=09=09\ >=20 > Good catch. As I understand the old code tries to avoid processing of=20 > var and val expressions twice. It looks it could be kept for val at=20 > least. Just keep if condition as in old code and fix the last line=20 > above: > =C2=A0=C2=A0=C2=A0 (var) =3D val_; > Since var_ and val_are local variables there is no necessity to=20 > enclose it in parenthesis (but does not harm if done). > var_ may be really removed since since resulting code will use it only=20 > once. I think there is a solution to avoid multiple evaluations of parameters: // var is definitely an lvalue, so its address can definitely be taken #def= ine ASSIGN_UNLESS_EQUAL(var, val) do { \ =09typeof(var) *const var_ =3D &(var); \ =09typeof(val) const val_ =3D (val); \ =09if (*var_ !=3D val_) \ =09=09*var_ =3D val_; \ } while (0) The solution relies on the compiler optimizer. Here is the comparison of wh= at the variants are compiled into: https://urldefense.com/v3/__https://godbolt.org/z/nnvq5q__;!!GqivPVa7Brio!M= JZCNAgcCHB5A_T1mI2aA-2F9wvqh_WOfzkeN0IbDnsZSlNIPWqDF0b4YDmTc19HcA$