On Thu, 2010-08-26 at 14:03 +0200, Stanislaw Gruszka wrote:
> > Since we've had locking problems in such situations a lot, I'm just
> > going to allow mac80211 to call scan_completed() from any context.
> > That'll get rid of all the problems with the mutx here, so that you can
> > move this code into a helper function (based on your description, I
> > suspect I'll see it again in the patch)
>
> This is mutex recursion problem, scan_completed() call
> ieee80211_hw_config() -> iwl_mac_config() -> mutex_lock(&priv->mutex).
> I should put short comment about that. Even on possibly removal
> of ieee80211_hw_config (for example when aborted==true) we still
> can have possible deadlock, because of mac80211 local->mtx and
> iwlwifi priv->mutex locking ordering.
Yeah, but I see you found my other patch now, which makes
ieee80211_scan_completed() really callable everywhere :)
> > > + if (priv->is_internal_short_scan &&
> > > + priv->scan_request == NULL)
> > > + ret = 0;
> >
> > Is that && really correct? It's just an extra check, right? I mean,
> > scan_request is always NULL for internal short scans...
> Nope, your commit f84b29ec0a1ab767679d3f2428877b65f94bc3ff changed
> that :-)
D'oh! You're right.
> > Hmm, an only tangentially related question: do we really need to do all
> > these atomic bit operations? We hold the mutex everywhere anyway, no?
>
> No. This my way to write things in one line instead of two, but perhaps
> two lines version should be used to not confuse readers.
No, I wasn't thinking of set_bit/clear_bit etc. but rather using bool
values... Don't worry about that though, I'll go through at some point
and check all the status bits.
> > > + if (test_bit(STATUS_SCAN_HW, &priv->status)) {
> > > + /* just live w/ bad key and rely briefly on SW decryption */
> > > return;
> > > }
> > > + /* XXX: race condition: nothing prevent to start HW scanning now */
> >
> > TBH, I don't even understand why we need to cancel the scan here. We're
> > just updating a key ... and we don't really do that while scanning
> > anyway since it's triggered only by receiving frames successfully...
>
> I don't understand this also.
It's probably some hw requirement that failed to make it into a
comment ... Wey-yi?
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
