From: Nikolay Aleksandrov <nikolay@redhat.com>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, andy@greyhouse.net, fubar@us.ibm.com
Subject: [PATCH net-next v3 0/5] bonding: groundwork and initial conversion to RCU
Date: Thu, 1 Aug 2013 16:54:46 +0200 [thread overview]
Message-ID: <1375368891-9979-1-git-send-email-nikolay@redhat.com> (raw)
From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@redhat.com>
Hello,
This patchset aims to lay the groundwork, and do the initial conversion to
RCUism. I decided that it'll be much better to make the bonding RCU
conversion gradual, so patches can be reviewed and tested better rather
than having one huge patch (which I did in the beginning, before this).
The first patch is straightforward and it converts the bonding to the
standard list API, simplifying a lot of code, removing unnecessary local
variables and allowing to use the nice rculist API later. It also takes
care of some minor styling issues (re-arranging local variables longest ->
shortest, removing brackets for single statement if/else, leaving new line
before return statement etc.).
The second patch simplifies the conversion by removing unnecessary
read_lock(&bond->curr_slave_lock) in xmit paths that are to be converted
later, because we only care if the pointer is NULL or a slave there, since
we already have bond->lock the slave can't go away.
The third patch simplifies the broadcast xmit function by removing
the use of curr_active_slave and converting to standard list API. Also this
design of the broadcast xmit function avoids a subtle double packet tx race
when converted to RCU.
The fourth patch factors out the code that transmits skb through a slave
with given id (i.e. rr_tx_counter in rr mode, hashed value in xor mode) and
simplifies the active-backup xmit path because bond_dev_queue_xmit always
consumes the skb. The new bond_xmit_slave_id function is used in rr and xor
modes currently, but the plans are to use it in 3ad mode as well thus it's
made global. I've left the function prototype to be 81 chars so I wouldn't
break it, if this is an issue I can always break it in more lines.
The fifth patch introduces RCU by converting attach/detach and release to
RCU. It also converts dereferencing of curr_active_slave to rcu_dereference
although it's not fully converted to RCU, that is needed for the converted
xmit paths. And it converts roundrobin, broadcast, xor and active-backup
xmit paths to RCU. The 3ad and ALB/TLB modes acquire read_lock(&bond->lock)
to make sure that no slave will be removed and to sync properly with
enslave and release as before.
This way for the price of a little complexity, we'll be able to convert
individual parts of the bonding to RCU, and test them easier in the
process. If this patchset is accepted in some form, I'll post followups
in the next weeks that gradually convert the bonding to RCU and remove the
need for the rwlocks.
For performance notes please refer to patch 5 (RCU conversion one).
v3 summary:
patch 1 - fix 3ad bug where next_slave wasn't checked if we've crossed
to the first slave again (v2 introduced bug).
Note: I know that it's not necessary to check next_slave for NULL but
later when the mode is converted to RCU that'll be necessary so I put
it there from now
v2 summary:
patch 1 - new primitives as suggested, removal of slave_cnt for checking
if there're slaves in the bonding (using list_empty instead), using
bond_for_each_slave and introducing bond_for_each_slave_continue_reverse
for consistency, also making bond_next/prev_slave return NULL and
simplifying further, also make bond_for_each_slave_from check if pos is
NULL, also I think we fix an ALB bug in bond_alb_deinit_slave because
we have to check if any slaves have the released slave's MAC address, and
in case we have 2 slaves "> 1" would fail since this one has been already
detached and slave_cnt is == 1 exactly, so switch to !list_empty
patch 4 - don't set slave to NULL in roundrobin, it's not necessary
patch 5 - included the performance notes, converted curr_active_slave in
bond_sysfs.c to rcu_dereference, fixed a potential latency issue in slave
release because TX was unblocked after synchronize_rcu which might take a
while
Nikolay Aleksandrov (5):
bonding: convert to list API and replace bond's custom list
bonding: remove unnecessary read_locks of curr_slave_lock
bonding: simplify broadcast_xmit function
bonding: factor out slave id tx code and simplify xmit paths
bonding: initial RCU conversion
drivers/net/bonding/bond_3ad.c | 44 ++--
drivers/net/bonding/bond_alb.c | 57 +++--
drivers/net/bonding/bond_main.c | 433 +++++++++++++++-----------------------
drivers/net/bonding/bond_procfs.c | 12 +-
drivers/net/bonding/bond_sysfs.c | 62 +++---
drivers/net/bonding/bonding.h | 85 +++++---
6 files changed, 310 insertions(+), 383 deletions(-)
--
1.8.1.4
next reply other threads:[~2013-08-01 14:58 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-01 14:54 Nikolay Aleksandrov [this message]
2013-08-01 14:54 ` [PATCH net-next v3 1/5] bonding: convert to list API and replace bond's custom list Nikolay Aleksandrov
2013-08-01 14:54 ` [PATCH net-next v3 2/5] bonding: remove unnecessary read_locks of curr_slave_lock Nikolay Aleksandrov
2013-08-01 14:54 ` [PATCH net-next v3 3/5] bonding: simplify broadcast_xmit function Nikolay Aleksandrov
2013-08-01 14:54 ` [PATCH net-next v3 4/5] bonding: factor out slave id tx code and simplify xmit paths Nikolay Aleksandrov
2013-08-01 14:54 ` [PATCH net-next v3 5/5] bonding: initial RCU conversion Nikolay Aleksandrov
2013-08-01 23:48 ` [PATCH net-next v3 0/5] bonding: groundwork and initial conversion to RCU David Miller
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=1375368891-9979-1-git-send-email-nikolay@redhat.com \
--to=nikolay@redhat.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=netdev@vger.kernel.org \
/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).