[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <30789.1239819113@death.nxdomain.ibm.com>
Date: Wed, 15 Apr 2009 11:11:53 -0700
From: Jay Vosburgh <fubar@...ibm.com>
To: paul@...-scientist.net
cc: netdev@...r.kernel.org
Subject: Re: 2.6.27.18: bnx2/tg3: BUG: "scheduling while atomic" trying to ifenslave a second interface to my bond
Paul Smith <paul@...-scientist.net> wrote:
>On Tue, 2009-04-14 at 18:12 -0700, Jay Vosburgh wrote:
>> I think I know what's going on. I believe this patch will
>> resolve things, but I won't be able to test it until tomorrow. If you
>> want to test this, great; if you want to wait, that's fine too.
>
>Hi Jay; as I mentioned last night this patch is working fine for me so
>far.
Thanks for the test report.
>However, looking at the rest of this function it seems to me that there
>are other locking issues, at least based on the documentation in the
>header file:
>
> * Here are the locking policies for the two bonding locks:
> *
> * 1) Get bond->lock when reading/writing slave list.
> * 2) Get bond->curr_slave_lock when reading/writing bond->curr_active_slave.
> * (It is unnecessary when the write-lock is put with bond->lock.)
> * 3) When we lock with bond->curr_slave_lock, we must lock with bond->lock
> * beforehand.
>
>For example, don't you need to hold bond->curr_slave_lock at least
>around the "if (!bond->curr_active_slave)"? What about around the
>"bond_for_each_slave" loop?
>
>Many of the other functions, later, also seem to work with
>bond->curr_active_slave and they don't take this lock.
>
>Unless I'm missing something, I think there are still more problems in
>the locking in bond_alb_set_mac_address().
The various MAC manipulating functions are either called under
RTNL (as bond_alb_set_mac_address is) or take pains to acquire RTNL
before doing anything with the MAC. Also, the slave list and
curr_active_slave are mutexed by RTNL, so those inspections should be
safe.
I'm reasonably sure that the curr_slave_lock is superfluous
(which wasn't the case when it was originally introduced), but I haven't
had a chance to validate this. The locking has changed from what's
documented in the header file; RTNL wasn't used for this when that was
written.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists