Messages in this thread | | | Date | Sat, 20 May 2006 09:58:45 +0200 | From | Brice Goglin <> | Subject | Re: [PATCH 3/4] myri10ge - Driver core |
| |
Brice Goglin wrote: > Andi Kleen wrote: > >> On Friday 19 May 2006 12:00, Arnd Bergmann wrote: >> >> >>> On Friday 19 May 2006 04:25, Brice Goglin wrote: >>> >>> >>>> dev_mc_upload() from net/core/dev_mcast.c does >>>> >>>> spin_lock_bh(&dev->xmit_lock); >>>> __dev_mc_upload(dev); >>>> >>>> which calls dev->set_multicast_list(), which is >>>> myri10ge_set_multicast_list() >>>> >>>> which calls myri10ge_change_promisc >>>> >>>> which calls myri10ge_send_cmd >>>> >>>> >>> Hmm, if that is the only path where you call it, it may be >>> helpful to change myri10ge_change_promisc to call a special >>> atomic version of myri10ge_send_cmd then, while all others >>> use the regular version, which you can then convert to >>> do msleep as well. >>> >>> >> Or just drop the xmit lock while you do that. As long as you >> handle races with ->start_xmit yourself it's ok >> >> > > Looks like we can do that. >
Forget what I said. It's not that simple. __dev_mc_upload() is also called from igmp6_group_added() (through dev_mc_add()). In this path, there are two spin_lock_bh that are held:
igmp6_group_added(...) [...] spin_lock_bh(&mc->mca_lock); [...] dev_mc_add(...)
dev_mv_add(..) [...] spin_lock_bh(&dev->xmit_lock); [...] __dev_mc_upload(...)
We have actually seen this path being taken by adding a if (in_interrupt()) { printk("bad bad bad\n"); dump_stack(); } in our myri10ge_send_cmd(...).
We are going to add a "sleepable" parameter to myri10ge_send_cmd() so that only set_multicast_list() will use it with our previous udelay loop.
Brice
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |