David, all. My apologies. I'm tired and I don't have the energy to
grok yet another part of the kernel right now. So I can not
productively participate in this discussion.
I do agree that the locking below dev_unicast_add() that was exposed
by the macvlan driver is unmaintainable even if it is
From: [EMAIL PROTECTED] (Eric W. Biederman)
Date: Thu, 11 Oct 2007 10:33:39 -0600
> Having ASSERT_RTNL warn if you were sleeping does not seem
> intuitive from the name.
>
> This instance of convoluted locking seems like a complete
> one off to me, and if it will warn about other constructs
> cur
Herbert Xu <[EMAIL PROTECTED]> writes:
> Well thanks to that warning we're on our way of improving the
> code that triggered it in such a way that this warning will soon
> go silent.
>
> That's precisely the reason why I object to having this warning
> removed. Now you have a good point that this
On Thu, Oct 11, 2007 at 02:23:35AM -0600, Eric W. Biederman wrote:
>
> > So I would object to a patch that caused the RTNL_ASSERT to not
> > warn about being called in an atomic context.
>
> ASSERT_RTNL does not warn about being called in an atomic context
> today!
Well it did didn't it or we wou
Herbert Xu <[EMAIL PROTECTED]> writes:
> On Thu, Oct 11, 2007 at 12:57:31AM -0600, Eric W. Biederman wrote:
>>
>> 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 macvl
On Thu, Oct 11, 2007 at 12:57:31AM -0600, Eric W. Biederman wrote:
>
> 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.
As I've already said we sh
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
>>
>> W
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 i
Herbert Xu wrote:
> On Tue, Oct 02, 2007 at 05:29:11PM +0200, Patrick McHardy wrote:
>
>>I think this doesn't completely fix it, when dev_unicast_add is
>>interrupted by dev_mc_add before the unicast changes are performed,
>>they will get committed in the dev_mc_add context, so we might still
>>ca
On Tue, Oct 02, 2007 at 05:29:11PM +0200, Patrick McHardy wrote:
>
> I think this doesn't completely fix it, when dev_unicast_add is
> interrupted by dev_mc_add before the unicast changes are performed,
> they will get committed in the dev_mc_add context, so we might still
> call change_flags with
On Tue, 2 Oct 2007, Herbert Xu wrote:
On Sun, Sep 30, 2007 at 05:47:30PM +0200, Patrick McHardy wrote:
I'm a bit uncomfortable with having a function (change_flags)
that's sometimes in a sleepable context and sometimes running
with BH disabled.
So how about splitting up the unicast/multicast
On Sun, Sep 30, 2007 at 05:47:30PM +0200, Patrick McHardy wrote:
>
> In the IPv6 case we're only changing the multicast list,
> so we're never calling into __dev_set_promiscuity.
>
> I actually even added a comment about this :)
>
> /* Unicast addresses changes may only happen under the r
Herbert Xu wrote:
> On Sat, Sep 29, 2007 at 05:32:41PM +0200, Patrick McHardy wrote:
>
>>For unicast addresses its not strictly necessary since they may
>>only be changed under the RTNL anyway. The reason why it takes
>>the tx_lock is for consistency with multicast address handling,
>>which can't
On Sat, Sep 29, 2007 at 11:18:18AM -0600, Eric W. Biederman wrote:
>
> 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.
Provided that you add a might_sle
On Sat, Sep 29, 2007 at 05:32:41PM +0200, Patrick McHardy wrote:
>
> For unicast addresses its not strictly necessary since they may
> only be changed under the RTNL anyway. The reason why it takes
> the tx_lock is for consistency with multicast address handling,
> which can't rely on the RTNL sinc
Eric W. Biederman wrote:
> 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.
Agreed. I actually have an identical patch somewhere that
I wanted to submit
Herbert Xu <[EMAIL PROTECTED]> writes:
> Eric W. Biederman <[EMAIL PROTECTED]> 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
Herbert Xu wrote:
> Eric W. Biederman <[EMAIL PROTECTED]> 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
>>al
Eric W. Biederman <[EMAIL PROTECTED]> 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
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 clear
20 matches
Mail list logo
