Garrett,
To prevent deadlocks and recursive locks, we need to have a strict lock
ordering
across layers. Currently the vast majority seem to be use top -> bottom
ordering
while holding locks across layers, since most of the control paths go
top down.
A simple rule set would be
* Each layer is free to decide on the locking hierarchy used with
respect to locks
it owns, i.e. the locks in its own layer
* There is a single global order for holding locks across layers, i.e.
top to bottom.
So layer A can hold a lock and call into layer B, only if A is above B.
Trying to fine grain the above, and defining the order on a per entry
point basis makes the
permutations unmanageable. Note that the problem you see is not driver
specific. Every
layer has to handle the problem in the upcall direction.
With the above rules, no layer (including the driver) may hold any lock
in the upcall direction.
Thirumalai
Garrett D'Amore wrote:
Eric Cheng wrote:
Garrett D'Amore wrote:
So, given this, it is it safe for me to just call
mac_{link,tx_notify() while holding driver locks?
I would recommend that you not do this even though the code appears
to be safe for now. the reason is that you do not know what the
notify callback is going to do. it may somehow loopback down to the
driver to acquire the same lock you're holding.
I realize that is a concern, which is why I'm asking here. If all the
notify does (and ever will do) is fire off a taskq, then its going to
be safe. But if it loops back (or ever will), then its not.
Or must I go to contortions (such as adding my own driver taskq, or
a soft interrupt) to ensure that I never call this while holding a
driver specific lock?
what's the problem with releasing the lock, then call mac_link_notify()?
You cant just "release" the lock in the middle of your code, always.
There are often considerations that have to be made about whether it
is actually _safe_ to drop the lock. Assumptions that you made and
were protected by the lock, are no longer true after the lock is
dropped. (Of course, if you're doing the notify at nearly the same
place as you drop the lock anyway, its easy. But if the notify is
deep inside nested code, its much harder to do safely.)
For example, on the tx hot path, I might find that I'm out of
descriptors. If that happens, I need to attempt to reclaim them. If
I do reclaim some, and if the mac layer was previously suspended due
to flow control, then I need to notify the mac layer.
All this happens in the hot path, and dropping and reacquiring the
lock here would be a significant pain in the butt.
I can try to track whether I've reclaimed resources or not, and do the
notify later after the lock is reclaimed, but that significantly
complicates the code, separates the action of notification far from
the point when I've discovered that I should do the notification, and
is thus likely to be a source of errors, particularly when the code is
maintained by someone else who may not see the bigger picture.
Its much simpler if I have just one call to mac_tx_update, at the
point at which I've discovered that I need to do it, rather than
deferring it to some other point after I've dropped the lock.
Will this guarantee hold true going forward? Obviously, since it
does a lot to simplify driver design, and eliminate extra context
switches, I'd really prefer to be able to rely on this behavior.
we're redesigning nemo's locking as part of the crossbow project. you
will hear more about this in several weeks time. for now, I'd suggest
that you try not to hold locks across any mac_* interface. if there
are issues with doing this, let us know and we'll take your needs
into account in our new design.
See the above. I'd really, really like to have mac_xxx_update()
continue to use the asynchronous taskq notification and therefore be
safe to call with locks held, if at all possible. Obviously the other
mac functions (e.g. mac_rx, etc.) really do need to be called without
any locks held.
-- Garrett
_______________________________________________
networking-discuss mailing list
[email protected]
_______________________________________________
networking-discuss mailing list
[email protected]
