David Miller <[EMAIL PROTECTED]> writes: > From: [EMAIL PROTECTED] (Eric W. Biederman) > Date: Fri, 28 Sep 2007 18:59:08 -0600 > >> >> Currently we have the call path: >> macvlan_open -> dev_unicast_add -> __dev_set_rx_mode -> >> __dev_set_promiscuity -> ASSERT_RTNL -> mutex_trylock >> >> When mutex debugging is on taking a mutex complains if we are not >> allowed to sleep. At that point we have called netif_tx_lock_bh >> so we are clearly not allowed to sleep. Arguably this is not a >> problem for mutex_trylock. >> >> However we can avoid the complaint and make the ASSERT_RTNL code >> cheaper, faster and more obvious by simply calling mutex_is_locked. >> >> So this patch adds rtnl_is_locked (which does mutex_is_locked on >> the rtnl_mutex) and changes ASSERT_RTNL to use that. >> >> Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]> > > There was a lot of discussion about how to do this right and > therefore you'll need to resubmit all of this with this > discussioned issues addressed.
Thanks for the update on where this patch sits. My understanding is that the patch as I submitted it is correct. The code path that sparked this patch has been seen to be extremely convoluted from a locking perspective. Herbert and Patrick have been discussing that code path and it was my impression they were working together to figure out how to refactor that code path to make the locking simpler. There are enough convoluted details that I do not feel comfortable refactoring that code path. There was a practical suggestion by Herbert that ASSERT_RTNL have a might_sleep() added. That suggestion will currently result in ASSERT_RTNL firing unnecessarily from the macvlan_open code path. So I do not feel comfortable adding a might_sleep() into ASSERT_RTNL, as it appears that it will warn on currently correct code. Even if that code has code has nearly incomprehensible locking. Eric - 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
