From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [PATCH] rtnl: Simplify ASSERT_RTNL Date: Sat, 29 Sep 2007 11:18:18 -0600 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: davem@davemloft.net, netdev@vger.kernel.org To: Herbert Xu Return-path: Received: from ebiederm.dsl.xmission.com ([166.70.28.69]:51279 "EHLO ebiederm.dsl.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755081AbXI2RTo (ORCPT ); Sat, 29 Sep 2007 13:19:44 -0400 In-Reply-To: (Herbert Xu's message of "Sat, 29 Sep 2007 12:31:14 +0800") Sender: netdev-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Herbert Xu writes: > Eric W. Biederman wrote: >> >> 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. > > Actually holding the TX lock here is a bug. We're going to > call down into the hardware with __dev_set_promiscuity, which > may sleep (think USB NICs), so we definitely shouldn't be holding > any spin locks. Regardless of the correctness of where we have ASSERT_RTNL. I think not actually taking the mutex on the assertion failure path (just so we can release it), is still a good deal regardless. For this particular call site clearly we need to look at what is happening a little more. The obvious thing would be to add an explicit might_sleep if we are calling code that can sleep. Eric