On Wed, Dec 12, 2007 at 09:46:55AM -0800, Jay Vosburgh wrote: > Herbert Xu <[EMAIL PROTECTED]> wrote: > > >> diff -puN drivers/net/bonding/bond_sysfs.c~bonding-locking-fix > >> drivers/net/bonding/bond_sysfs.c > >> --- a/drivers/net/bonding/bond_sysfs.c~bonding-locking-fix > >> +++ a/drivers/net/bonding/bond_sysfs.c > >> @@ -1111,8 +1111,6 @@ static ssize_t bonding_store_primary(str > >> out: > >> write_unlock_bh(&bond->lock); > >> > >> - rtnl_unlock(); > >> - > > > >Looking at the changeset that added this perhaps the intention > >is to hold the lock? If so we should add an rtnl_lock to the start > >of the function. > > Yes, this function needs to hold locks, and more than just > what's there now. I believe the following should be correct; I haven't > tested it, though (I'm supposedly on vacation right now). > > The following change should be correct for the > bonding_store_primary case discussed in this thread, and also corrects > the bonding_store_active case which performs similar functions. > > The bond_change_active_slave and bond_select_active_slave > functions both require rtnl, bond->lock for read and curr_slave_lock for > write_bh, and no other locks. This is so that the lower level > mode-specific functions can release locks down to just rtnl in order to > call, e.g., dev_set_mac_address with the locks it expects (rtnl only). > > Signed-off-by: Jay Vosburgh <[EMAIL PROTECTED]>
This looks good to me as well.... Signed-off-by: Andy Gospodarek <[EMAIL PROTECTED]> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
