On 06/02/10 02:42, Jay Vosburgh wrote: > Cong Wang<amwang@xxxxxxxxxx> wrote: > >> On 06/01/10 03:08, Flavio Leitner wrote: >>> On Mon, May 31, 2010 at 01:56:52PM +0800, Cong Wang wrote: >>>> Hi, Flavio, >>>> >>>> Please use the attached patch instead, try to see if it solves >>>> all your problems. >>> >>> I tried and it hangs. No backtraces this time. >>> The bond_change_active_slave() prints before NETDEV_BONDING_FAILOVER >>> notification, so I think it won't work. >> >> Ah, I thought the same. >> >>> >>> Please, correct if I'm wrong, but when a failover happens with your >>> patch applied, the netconsole would be disabled forever even with >>> another healthy slave, right? >>> >> >> Yes, this is an easy solution, because bonding has several modes, >> it is complex to make netpoll work in different modes. > > If I understand correctly, the root cause of the problem with > netconsole and bonding is that bonding is, ultimately, performing > printks with a write lock held, and when netconsole recursively calls > into bonding to send the printk over the netconsole, there is a deadlock > (when the bonding xmit function attempts to acquire the same lock for > read). Yes. > > You're trying to avoid the deadlock by shutting off netconsole > (permanently, it looks like) for one problem case: a failover, which > does some printks with a write lock held. > > This doesn't look to me like a complete solution, there are > other cases in bonding that will do printk with write locks held. I > suspect those will also hang netconsole as things exist today, and won't > be affected by your patch below. I can expect that, bonding modes are complex. > > For example: > > The sysfs functions to set the primary (bonding_store_primary) > or active (bonding_store_active_slave) options: a pr_info is called to > provide a log message of the results. These could be tested by setting > the primary or active options via sysfs, e.g., > > echo eth0> /sys/class/net/bond0/bonding/primary > echo eth0> /sys/class/net/bond0/bonding/active > > If the kernel is defined with DEBUG, there are a few pr_debug > calls within write_locks (bond_del_vlan, for example). > > If the slave's underlying device driver's ndo_vlan_rx_register > or ndo_vlan_rx_kill_vid functions call printk (and it looks like some do > for error cases, e.g., igbvf, ehea, enic), those would also presumably > deadlock (because bonding holds its write_lock when calling the ndo_ > vlan functions). > > It also appears that (with the patch below) some nominally > normal usage patterns will immediately disable netconsole. The one that > comes to mind is if the primary= option is set (to "eth1" for this > example), but that slave not enslaved first (the slaves are added, say, > eth0 then eth1). In that situation, when the primary slave (eth1 here) > is added, the first thing that will happen is a failover, and that will > disable netconsole. > Thanks for your detailed explanation! This is why I said bonding is complex. I guess we would have to adjust netpoll code for different bonding cases, one solution seems not fix all. I am not sure how much work to do, since I am not familiar with bonding code. Maybe Andy can help? For the previous patch, it at least can make Flavio happy. :) Thanks! _______________________________________________ Bridge mailing list Bridge@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/bridge
