Re: [PATCH RFC] net/bonding: announce fail-over for the active-backup mode

[Or Gerlitz <ogerlitz@voltaire.com>]

Jay Vosburgh wrote:
> Or Gerlitz <ogerlitz@voltaire.com> wrote:
>
> 	Philosophically speaking, I don't see a problem with adding a
> notifier like this, but others higher in the food chain may have
> thoughts.
OK. I will be happy to get more feedback.

> 	Your case is similar: you want to issue a notifier call during
> an active-backup failover, so that notifier call will have to be made
> holding RTNL and no other locks.
>
> 	I think the most maintainable way to do that is to convert the
> remaining callers of bond_change_active_slave to hold the correct set of
> locks, and then have bond_change_active_slave drop down to just RTNL at
> the appropriate place to make the notifier call.  That may not be as
> simple as it sounds, as it may open race windows.
>
Lets say that everyone calls bond_change_active_slave with the correct 
locks taken and the code that delivers the event, unlocks these two 
locks, call to the notifier chain through dev_set_xxx() and then locks 
them again. These locks were there in the first place to protect on 
something, so generally speaking I don't see why its allowed to unlock 
them for some window of time... is it some "best effort" compromise?

Second, if it makes sense to have this window at time where the other 
two locks are not taken and only the RTNL one is taken. Is there any 
reason I can't take the approach of bond_alb_handle_active_change() 
which as you pointed out, releases the locks, delivers the event and 
take them again? is there something different between the possible calls 
under the active-backup mode vs the ALB mode that requires to do this 
deeper fix?

>> bonding: bond0: link status definitely down for interface ib0, disabling it
>> bonding: bond0: making interface ib1 the new active one.
>> BUG: scheduling while atomic: bond0/14237/0x10000100
> It's from the call to nlmsg_new (an inline that calls alloc_skb) in rtmsg_ifinfo, which allocates at GFP_KERNEL.  As I recall, there are other similar cases, so it's not simply a matter of changing rtmsg_ifinfo.  The notifier calls have to happen with RTNL and no other locks.
Understood, thanks for clarifying this.

Or.