From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 43B2429CB for ; Fri, 10 Jun 2016 20:24:54 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP; 10 Jun 2016 11:24:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,451,1459839600"; d="scan'208";a="995187947" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga002.jf.intel.com with ESMTP; 10 Jun 2016 11:24:52 -0700 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by IRSMSX154.ger.corp.intel.com (163.33.192.96) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 10 Jun 2016 19:24:51 +0100 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.183]) by irsmsx112.ger.corp.intel.com ([169.254.1.18]) with mapi id 14.03.0248.002; Fri, 10 Jun 2016 19:24:51 +0100 From: "Iremonger, Bernard" To: "Richardson, Bruce" CC: "dev@dpdk.org" , "Doherty, Declan" , "Ananyev, Konstantin" Thread-Topic: [dpdk-dev] [PATCH v2 0/6] bonding: locks Thread-Index: AQHRt20ma1Q3SxxN8k2wu3EVzEjs4Z/izegAgABMxYA= Date: Fri, 10 Jun 2016 18:24:50 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C21A038336@IRSMSX108.ger.corp.intel.com> References: <1462461300-9962-1-git-send-email-bernard.iremonger@intel.com> <1464280727-25752-1-git-send-email-bernard.iremonger@intel.com> <20160610144542.GA16264@bricha3-MOBL3> In-Reply-To: <20160610144542.GA16264@bricha3-MOBL3> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjU4Y2VhZWYtN2I1ZS00NWQ1LWExNTAtYzNlZmVkN2Y0OTU1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InRldm9KWlp1UldmTnhrUnNaY2dJRmR3eHNXRkJKUFBCSmQ5SGcwY0RtOEE9In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 0/6] bonding: locks X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Jun 2016 18:24:54 -0000 Hi Bruce, > -----Original Message----- > From: Richardson, Bruce > Sent: Friday, June 10, 2016 3:46 PM > To: Iremonger, Bernard > Cc: dev@dpdk.org; Doherty, Declan ; Ananyev, > Konstantin > Subject: Re: [dpdk-dev] [PATCH v2 0/6] bonding: locks >=20 > On Thu, May 26, 2016 at 05:38:41PM +0100, Bernard Iremonger wrote: > > Add spinlock to bonding rx and tx queues. > > Take spinlock in rx and tx burst functions. > > Take all spinlocks in slave add and remove functions. > > With spinlocks in place remove memcpy of slaves. > > > > Changes in v2: > > Replace patch 1. > > Add patch 2 and reorder patches. > > Add spinlock to bonding rx and tx queues. > > Take all spinlocks in slave add and remove functions. > > Replace readlocks with spinlocks. > > > > Bernard Iremonger (6): > > bonding: add spinlock to rx and tx queues > > bonding: grab queue spinlocks in slave add and remove > > bonding: take queue spinlock in rx/tx burst functions > > bonding: add spinlock to stop function > > bonding: add spinlock to link update function > > bonding: remove memcpy from burst functions > > > > drivers/net/bonding/rte_eth_bond_api.c | 52 +++++++- > > drivers/net/bonding/rte_eth_bond_pmd.c | 196 > ++++++++++++++++++----------- > > drivers/net/bonding/rte_eth_bond_private.h | 4 +- > > 3 files changed, 173 insertions(+), 79 deletions(-) > > > > -- >=20 > The patches in this set are missing any explanation for the reasons behin= d > each patch. The commit messages are empty for every patch. >=20 > I'm also concerned at the fact that this patchset is adding lock operatio= ns all > over the bonding PMD. While other PMDs need synchronization between > control plane and data plane threads so that e.g. you don't do IO on a > stopped port, they don't use locks so as to get max performance. Nowhere > in the patchset is there an explanation as to why bonding is so different= that > it needs locks where other PMDs can do without them. This should also be > explained in each individual patch as to why the area covered by the patc= h > needs locks in this PMD (again, compared to other PMDs) >=20 > Thanks, > /Bruce I will be sending a v3 for this patchset. The empty commit messages were an oversight on my part, this will be correc= ted in the v3. I will also try to explain why the locks are needed. Regards, Bernard.