All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ding Tianhong <dthxman@gmail.com>
To: David Miller <davem@davemloft.net>, dingtianhong@huawei.com
Cc: fubar@us.ibm.com, andy@greyhouse.net, nikolay@redhat.com,
	vfalico@redhat.com, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v5 0/11] bonding: rebuild the lock use for bond monitor
Date: Sat, 14 Dec 2013 18:02:25 +0800	[thread overview]
Message-ID: <52AC2CB1.9090102@gmail.com> (raw)
In-Reply-To: <20131214.020149.2022684556057698333.davem@davemloft.net>

于 2013/12/14 15:01, David Miller 写道:
> From: Ding Tianhong <dingtianhong@huawei.com>
> Date: Wed, 11 Dec 2013 12:59:52 +0800
> 
>> Now the bond slave list is not protected by bond lock, only by RTNL,
>> but the monitor still use the bond lock to protect the slave list,
>> it is useless, according to the Veaceslav's opinion, there were
>> three way to fix the protect problem:
>>
>> 1. add bond_master_upper_dev_link() and bond_upper_dev_unlink()
>>    in bond->lock, but it is unsafe to call call_netdevice_notifiers()
>>    in write lock.
>> 2. remove unused bond->lock for monitor function, only use the exist
>>    rtnl lock(), it will take performance loss in fast path.
>> 3. use RCU to protect the slave list, of course, performance is better,
>>    but in slow path, it is ignored.
>>
>> obviously the solution 1 is not fit here, I will consider the 2 and 3
>> solution. My principle is simple, if in fast path, RCU is better,
>> otherwise in slow path, both is well, but according to the Jay Vosburgh's
>> opinion, the monitor will loss performace if use RTNL to protect the all
>> slave list, so remove the bond lock and replace with RCU.
>>
>> The second problem is the curr_slave_lock for bond, it is too old and
>> unwanted in many place, because the curr_active_slave would only be
>> changed in 3 place:
>>
>> 1. enslave slave.
>> 2. release slave.
>> 3. change active slave.
>>
>> all above were already holding bond lock, RTNL and curr_slave_lock
>> together, it is tedious and no need to add so mach lock, when change
>> the curr_active_slave, you have to hold the RTNL and curr_slave_lock
>> together, and when you read the curr_active_slave, RTNL or curr_slave_lock,
>> any one of them is no problem.
>>
>> for the stability, I did not change the logic for the monitor,
>> all change is clear and simple, I have test the patch set for lockdep,
>> it work well and stability.
> 
> Series applied, thanks.
> --

Hi David:

The last version of the patchset is v6, and the v6 is a little different from v5,
I think you have missed the last version, and found you applied the v5 patchset.

Regards
Ding

> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  reply	other threads:[~2013-12-14 10:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-11  4:59 [PATCH net-next v5 0/11] bonding: rebuild the lock use for bond monitor Ding Tianhong
2013-12-14  7:01 ` David Miller
2013-12-14 10:02   ` Ding Tianhong [this message]
2013-12-15  3:24     ` David Miller
2013-12-15  3:28       ` Ding Tianhong

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=52AC2CB1.9090102@gmail.com \
    --to=dthxman@gmail.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=fubar@us.ibm.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@redhat.com \
    --cc=vfalico@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.