All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.