From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id DACF72BA6 for ; Fri, 10 Jun 2016 16:45:46 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 10 Jun 2016 07:45:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,450,1459839600"; d="scan'208";a="973025093" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.74]) by orsmga001.jf.intel.com with SMTP; 10 Jun 2016 07:45:45 -0700 Received: by (sSMTP sendmail emulation); Fri, 10 Jun 2016 15:45:42 +0025 Date: Fri, 10 Jun 2016 15:45:42 +0100 From: Bruce Richardson To: Bernard Iremonger Cc: dev@dpdk.org, declan.doherty@intel.com, konstantin.ananyev@intel.com Message-ID: <20160610144542.GA16264@bricha3-MOBL3> References: <1462461300-9962-1-git-send-email-bernard.iremonger@intel.com> <1464280727-25752-1-git-send-email-bernard.iremonger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1464280727-25752-1-git-send-email-bernard.iremonger@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) 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 14:45:47 -0000 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(-) > > -- The patches in this set are missing any explanation for the reasons behind each patch. The commit messages are empty for every patch. I'm also concerned at the fact that this patchset is adding lock operations 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 patch needs locks in this PMD (again, compared to other PMDs) Thanks, /Bruce