Hi Bruce, > -----Original Message----- > From: Richardson, Bruce > Sent: Friday, June 10, 2016 3:46 PM > To: Iremonger, Bernard <bernard.iremonger at intel.com> > Cc: dev at dpdk.org; Doherty, Declan <declan.doherty at intel.com>; Ananyev, > Konstantin <konstantin.ananyev at intel.com> > Subject: Re: [dpdk-dev] [PATCH v2 0/6] bonding: locks > > 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
I will be sending a v3 for this patchset. The empty commit messages were an oversight on my part, this will be corrected in the v3. I will also try to explain why the locks are needed. Regards, Bernard.
