From: Bruce Richardson <bruce.richardson@intel.com>
To: Bernard Iremonger <bernard.iremonger@intel.com>
Cc: dev@dpdk.org, declan.doherty@intel.com, konstantin.ananyev@intel.com
Subject: Re: [dpdk-dev] [PATCH v2 0/6] bonding: locks
Date: Fri, 10 Jun 2016 15:45:42 +0100 [thread overview]
Message-ID: <20160610144542.GA16264@bricha3-MOBL3> (raw)
In-Reply-To: <1464280727-25752-1-git-send-email-bernard.iremonger@intel.com>
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
next prev parent reply other threads:[~2016-06-10 14:45 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-05 15:14 [dpdk-dev] [PATCH 0/5] " Bernard Iremonger
2016-05-05 15:14 ` [dpdk-dev] [PATCH 1/5] bonding: replace spinlock with read/write lock Bernard Iremonger
2016-05-05 17:12 ` Stephen Hemminger
2016-05-06 10:32 ` Declan Doherty
2016-05-06 15:55 ` Stephen Hemminger
2016-05-13 17:10 ` Ananyev, Konstantin
2016-05-13 17:18 ` Ananyev, Konstantin
2016-05-26 16:24 ` Iremonger, Bernard
2016-05-05 15:14 ` [dpdk-dev] [PATCH 2/5] bonding: add read/write lock to rx/tx burst functions Bernard Iremonger
2016-05-05 15:14 ` [dpdk-dev] [PATCH 3/5] bonding: remove memcopy of slaves from rx/tx burst function Bernard Iremonger
2016-05-05 15:14 ` [dpdk-dev] [PATCH 4/5] bonding: add read/write lock to stop function Bernard Iremonger
2016-05-05 15:15 ` [dpdk-dev] [PATCH 5/5] bonding: add read/write lock to the link_update function Bernard Iremonger
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 0/6] bonding: locks Bernard Iremonger
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 1/6] bonding: add spinlock to rx and tx queues Bernard Iremonger
2016-06-10 18:12 ` Ananyev, Konstantin
2016-06-12 17:11 ` [dpdk-dev] [PATCH v3 0/4] bonding: locks Bernard Iremonger
2016-06-12 17:11 ` [dpdk-dev] [PATCH v3 1/4] bonding: add spinlock to rx and tx queues Bernard Iremonger
2016-06-12 17:11 ` [dpdk-dev] [PATCH v3 2/4] bonding: grab queue spinlocks in slave add and remove Bernard Iremonger
2016-06-12 17:11 ` [dpdk-dev] [PATCH v3 3/4] bonding: take queue spinlock in rx/tx burst functions Bernard Iremonger
2016-06-13 9:18 ` Bruce Richardson
2016-06-13 12:28 ` Iremonger, Bernard
2016-06-16 14:32 ` Bruce Richardson
2016-06-16 15:00 ` Thomas Monjalon
2016-06-16 16:41 ` Iremonger, Bernard
2016-06-16 18:38 ` Thomas Monjalon
2017-02-15 18:01 ` Ferruh Yigit
2017-02-16 9:13 ` Bruce Richardson
2017-02-16 11:39 ` Iremonger, Bernard
2017-02-20 11:15 ` Ferruh Yigit
2016-09-09 11:29 ` Ferruh Yigit
2016-06-12 17:11 ` [dpdk-dev] [PATCH v3 4/4] bonding: remove memcpy from " Bernard Iremonger
2016-09-11 12:39 ` Yuanhan Liu
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 2/6] bonding: grab queue spinlocks in slave add and remove Bernard Iremonger
2016-06-10 18:14 ` Ananyev, Konstantin
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 3/6] bonding: take queue spinlock in rx/tx burst functions Bernard Iremonger
2016-06-10 18:14 ` Ananyev, Konstantin
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 4/6] bonding: add spinlock to stop function Bernard Iremonger
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 5/6] bonding: add spinlock to link update function Bernard Iremonger
2016-05-26 16:38 ` [dpdk-dev] [PATCH v2 6/6] bonding: remove memcpy from burst functions Bernard Iremonger
2016-06-10 18:15 ` Ananyev, Konstantin
2016-06-10 14:45 ` Bruce Richardson [this message]
2016-06-10 18:24 ` [dpdk-dev] [PATCH v2 0/6] bonding: locks Iremonger, Bernard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160610144542.GA16264@bricha3-MOBL3 \
--to=bruce.richardson@intel.com \
--cc=bernard.iremonger@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).