From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0070.outbound.protection.outlook.com [104.47.2.70]) by dpdk.org (Postfix) with ESMTP id 211C41B4C5 for ; Wed, 10 Oct 2018 12:06:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=dxnadwlerw7MbKPholZYnW1P7feCJ3/EIuGhRrQxY60=; b=BRxE8OLlEJJtAxF7Pm4BB0ikOXgeZOmlHbAbAppRaQA/9NxuppCZncZdryVhMj+MQ7qkFwn14k8oDwVUiJpkVaeH/+yQqrjIYJe+M6zEYKPoXSZiiwqL7qWZTWCmxZMDzn3SV8ENA1uDgvUpz3Xg3cblPLOKi/k2Qwe2MopELgE= Received: from VI1PR08MB3167.eurprd08.prod.outlook.com (52.133.15.142) by VI1PR08MB3645.eurprd08.prod.outlook.com (20.177.61.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1207.23; Wed, 10 Oct 2018 10:06:29 +0000 Received: from VI1PR08MB3167.eurprd08.prod.outlook.com ([fe80::4c13:b1f:ad01:86d7]) by VI1PR08MB3167.eurprd08.prod.outlook.com ([fe80::4c13:b1f:ad01:86d7%4]) with mapi id 15.20.1228.020; Wed, 10 Oct 2018 10:06:29 +0000 From: "Gavin Hu (Arm Technology China)" To: "Phil Yang (Arm Technology China)" , Stephen Hemminger CC: "dev@dpdk.org" , "jerin.jacob@caviumnetworks.com" , Honnappa Nagarahalli , Ola Liljedahl , "ferruh.yigit@intel.com" Thread-Topic: [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo synchronization Thread-Index: AQHUXub5zinpxPaBNkq5MLOshY7LvKUV5MoAgAJdFICAAAEO4A== Date: Wed, 10 Oct 2018 10:06:28 +0000 Message-ID: References: <1537364560-4124-1-git-send-email-phil.yang@arm.com> <1538989906-8349-1-git-send-email-phil.yang@arm.com> <1538989906-8349-2-git-send-email-phil.yang@arm.com> <20181008145308.0fbc64de@xeon-e3> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Gavin.Hu@arm.com; x-originating-ip: [113.29.88.7] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; VI1PR08MB3645; 6:5ESqRDuJI9HtUH53SJ24Krknx8PV6HV+sH45Xpwr1J5Fq8iN02mcgPE+mY9aMrF7kH9wZ8ye1VOu8+lvQ+zL6wvZcBwUq31dIq+8ZLJkX1IzaBRFyn7VxspoQ70LMUMwVVsgk6lT+T06I2AvBIzKtJjX7FTpU96mexLTVwFxtY2uumnHaWKdVksFqkZ3RWCCZ/hDeI48txfGYhHNTEqj841ZNnOQgc1r7idjSRyB8JZle/Eud1Bun244JOUAN5LOcgjryM4mey42v9zeoPsBon1zXlXfKXp22RP+6kxtbefpqAJy/6iWgizSbPPydTxIi8yBZUGVUV9Nv0ApahDf55JP9ZIq53Bn7UWCft9CYQmAKDamJweJLtbnA7BK9hnl4AUfUkIml/z2q65uro7q9uzXOm8RPKGHUIvc3ui/otv6qSrCxTiNUhTeUuYwYtUWeg/KrXXFpLrkTUncUQe07A==; 5:pPsoOoI6uhr4Cz20RdRN51i+nqundY2GPDQvu0czIG/PBg258mjBck8sdDhgC1RbSrI40DDYs5LB2G1bmKTie86xSjj1yv/GO3lqHX/3+MgOlewcgkBvK7LzCivKRi95PvUmdUohLhmAu6DIhPBuwU4pGfIp02y1HAxGFW1L5tA=; 7:kQ3OpwQriZArmZJ+XbOPJ+nzPL1Lq+ykT15fPvwYZkHiUQqtwML6dGTsta/vxrgJV1Wt03IPvd85kjgVmXqa4rKlxRS8q8tiqxaB4BU6DWaX+rHKHfYgj/AG/8JlD9PDyhZZ5ArteoUQD70Ui+U6F9+aUb8NVNxnzpFB9ebNllDNmHgCR4/zErS+ttmiRO7Q4EUoR+Mzb/y3QKolbw+AciJpr9dbpJal/vRGQrCWaxslojqkkf3pG/roffRfvD6X x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR; x-ms-office365-filtering-correlation-id: 74143779-09ff-44af-3c54-08d62e980c04 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:VI1PR08MB3645; x-ms-traffictypediagnostic: VI1PR08MB3645: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699)(180628864354917); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3231355)(944501410)(52105095)(3002001)(6055026)(149066)(150057)(6041310)(20161123560045)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(201708071742011)(7699051)(76991055); SRVR:VI1PR08MB3645; BCL:0; PCL:0; RULEID:; SRVR:VI1PR08MB3645; x-forefront-prvs: 08213D42D3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(136003)(376002)(396003)(366004)(346002)(13464003)(189003)(199004)(40434004)(33656002)(110136005)(25786009)(53936002)(186003)(6116002)(476003)(3846002)(26005)(14454004)(446003)(6246003)(55016002)(5250100002)(2906002)(9686003)(6436002)(54906003)(76176011)(7696005)(55236004)(99286004)(11346002)(6506007)(102836004)(93886005)(478600001)(66066001)(8676002)(68736007)(53546011)(105586002)(8936002)(2900100001)(106356001)(7736002)(256004)(305945005)(81166006)(81156014)(71190400001)(71200400001)(4326008)(5024004)(14444005)(74316002)(5660300001)(86362001)(316002)(97736004)(229853002)(72206003)(486006); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR08MB3645; H:VI1PR08MB3167.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: EjYPKHGgxTSCO2GcxQQxVkXykpbFxNUMjTafulPpMmFIXaCLNJiW87kWiVHvF0odOGVSAmfEuvaundAM23rfxi5xG2/kahxsLwSA7TPMEJULYuTDSCLscuflqxPgCOUJt9X0OZ/5o1n+sGxi3LBg7UEuX2B28pNTSDcEXs6lDwnLWqSqyhhQiUMI69HJW+0rKSxBVoI0gKVes7FJ8ZhRmtK9CiJVolJ4BYF1G2QCHqilnmfTuAV0rJ3DkwQ+JAQGftXwWKVmd364/zlCA7a1f0nwcmnaNwViNK1QTw6NBXeS3pi8liiVHlMOVRvlU79035+hmaSJp4CpwDuN8WyUVny+Ici77VQ8c1Dgaw3jWg4= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 74143779-09ff-44af-3c54-08d62e980c04 X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Oct 2018 10:06:28.9413 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3645 Subject: Re: [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo synchronization 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: Wed, 10 Oct 2018 10:06:31 -0000 > -----Original Message----- > From: Phil Yang (Arm Technology China) > Sent: Wednesday, October 10, 2018 5:59 PM > To: Stephen Hemminger > Cc: dev@dpdk.org; jerin.jacob@caviumnetworks.com; Gavin Hu (Arm > Technology China) ; Honnappa Nagarahalli > ; Ola Liljedahl ; > ferruh.yigit@intel.com > Subject: RE: [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo synchronization > > Hi Hemminger, > > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Tuesday, October 9, 2018 5:53 AM > > To: Phil Yang (Arm Technology China) > > Cc: dev@dpdk.org; jerin.jacob@caviumnetworks.com; Gavin Hu (Arm > > Technology China) ; Honnappa Nagarahalli > > ; Ola Liljedahl ; > > ferruh.yigit@intel.com > > Subject: Re: [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo > > synchronization > > > > On Mon, 8 Oct 2018 17:11:44 +0800 > > Phil Yang wrote: > > > > > diff --git a/lib/librte_kni/rte_kni_fifo.h > > > b/lib/librte_kni/rte_kni_fifo.h index ac26a8c..70ac14e 100644 > > > --- a/lib/librte_kni/rte_kni_fifo.h > > > +++ b/lib/librte_kni/rte_kni_fifo.h > > > @@ -28,8 +28,9 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void > > > **data, unsigned num) { > > > unsigned i =3D 0; > > > unsigned fifo_write =3D fifo->write; > > > -unsigned fifo_read =3D fifo->read; > > > unsigned new_write =3D fifo_write; > > > +rte_smp_rmb(); > > > +unsigned fifo_read =3D fifo->read; > > > > > > > The patch makes sense, but this function should be changed to match > > kernel code style. > > That means no declarations after initial block, and use 'unsigned int' > > rather than 'unsigned' > > > > Also. why is i initialized? Best practice now is to not do gratitious > > initialization since it defeats compiler checks for accidental use of > uninitialized variables. > > > > What makes sense is something like: > > > > kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num) { > > unsigned int i, fifo_read, fifo_write, new_write; > > > > fifo_write =3D fifo->write; > > new_write =3D fifo_write; > > rte_smb_rmb(); > > fifo_read =3D fifo->read; > > > > Sorry, blaming you for issues which are inherited from original KNI cod= e. > > Maybe someone should run kernel checkpatch (not DPDK checkpatch) on it > > and fix those. > > Thanks for your comment. > > I think I can submit a new separate patch to fix this historical issue. > > Thanks, > Phil Yang I advised a separate patch to make this patch to the point and clean. -Gavin IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.