From: Stefano Brivio <sbrivio@redhat.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Matteo Croce <mcroce@redhat.com>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: hung task in mac80211
Date: Wed, 6 Sep 2017 15:27:09 +0200 [thread overview]
Message-ID: <20170906152709.673f230d@elisabeth> (raw)
In-Reply-To: <1504704060.13457.20.camel@sipsolutions.net>
On Wed, 06 Sep 2017 15:21:00 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2017-09-06 at 15:19 +0200, Stefano Brivio wrote:
> > On Wed, 06 Sep 2017 14:48:35 +0200
> > Johannes Berg <johannes@sipsolutions.net> wrote:
> >
> > > I'll look in a bit - but
> > >
> > > > + mutex_unlock(&sta->ampdu_mlme.mtx);
> > > > ___ieee80211_stop_rx_ba_session(
> > > > sta, tid, WLAN_BACK_RECIPIENT,
> > > > WLAN_REASON_QSTA_TIMEOUT,
> > > > true);
> > >
> > > This already has three underscores so shouldn't drop.
> >
> > Right, of course.
> >
> > > [...]
> > > > + mutex_unlock(&sta->ampdu_mlme.mtx);
> > > > __ieee80211_start_rx_ba_session(sta, 0,
> > > > 0,
> > > > 0, 1, tid,
> > >
> > > maybe this one needs a ___ version then?
> >
> > Either that, or as it's a single call, perhaps just the following?
> > Matter of taste I guess...
>
> I don't think it's a matter of taste - for me, in principle, dropping
> locks for small sections of code where the larger section holds it is a
> bug waiting to happen. It may (may, I don't even know) be OK here, but
> in general it's something to avoid.
Yes, that was based on the assumption that the initial part of
__ieee80211_start_rx_ba_session() can't really affect the AMPDU
state-machine in any way.
But sure, one small change there in the future and the assumption
doesn't hold anymore.
--
Stefano
next prev parent reply other threads:[~2017-09-06 13:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-06 11:57 hung task in mac80211 Matteo Croce
2017-09-06 12:28 ` Christian Lamparter
2017-09-06 12:28 ` Christian Lamparter
2017-09-06 12:40 ` Stefano Brivio
2017-09-06 12:48 ` Johannes Berg
2017-09-06 13:03 ` Johannes Berg
2017-09-06 13:08 ` Sebastian Gottschall
2017-09-06 13:08 ` Sebastian Gottschall
2017-09-06 13:19 ` Stefano Brivio
2017-09-06 13:21 ` Johannes Berg
2017-09-06 13:27 ` Stefano Brivio [this message]
2017-09-06 13:30 ` Johannes Berg
2017-09-06 13:40 ` Stefano Brivio
2017-09-06 13:07 ` Matteo Croce
2017-09-06 13:07 ` Matteo Croce
2017-09-06 12:58 ` Johannes Berg
2017-09-06 14:27 ` Stefano Brivio
2017-09-06 14:30 ` Johannes Berg
2017-09-06 15:04 ` Matteo Croce
2017-09-06 15:11 ` Johannes Berg
2017-09-06 15:45 ` Sebastian Gottschall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170906152709.673f230d@elisabeth \
--to=sbrivio@redhat.com \
--cc=johannes@sipsolutions.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=mcroce@redhat.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.