From patchwork Wed Sep 6 14:27:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Brivio X-Patchwork-Id: 9940897 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5CA3A60216 for ; Wed, 6 Sep 2017 14:28:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4C63B28C09 for ; Wed, 6 Sep 2017 14:28:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 40C4928C14; Wed, 6 Sep 2017 14:28:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E3CF128C09 for ; Wed, 6 Sep 2017 14:28:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932564AbdIFO14 (ORCPT ); Wed, 6 Sep 2017 10:27:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56868 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932260AbdIFO1y (ORCPT ); Wed, 6 Sep 2017 10:27:54 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E7F655F7AC; Wed, 6 Sep 2017 14:27:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E7F655F7AC Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sbrivio@redhat.com Received: from elisabeth (unknown [10.33.36.62]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7797518C4B; Wed, 6 Sep 2017 14:27:52 +0000 (UTC) Date: Wed, 6 Sep 2017 16:27:48 +0200 From: Stefano Brivio To: Johannes Berg Cc: Matteo Croce , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: hung task in mac80211 Message-ID: <20170906162748.071b040e@elisabeth> In-Reply-To: <1504702728.13457.17.camel@sipsolutions.net> References: <1504702728.13457.17.camel@sipsolutions.net> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 06 Sep 2017 14:27:54 +0000 (UTC) Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 06 Sep 2017 14:58:48 +0200 Johannes Berg wrote: > +void __ieee80211_start_rx_ba_session(struct sta_info *sta, > + u8 dialog_token, u16 timeout, > + u16 start_seq_num, u16 ba_policy, u16 tid, > + u16 buf_size, bool tx, bool auto_seq) > +{ > + mutex_lock(&sta->ampdu_mlme.mtx); > + ___ieee80211_start_rx_ba_session(sta, dialog_token, timeout, > + start_seq_num, ba_policy, tid, > + buf_size, tx, auto_seq); > + mutex_unlock(&sta->ampdu_mlme.mtx); > +} > + Sorry for the extended bothering :) but here, you're extending quite a bit the scope of the lock also when__ieee80211_start_rx_ba_session() is called by ieee80211_process_addba_request(). No idea what the hit can be, but we can't safely assume it's nothing either. What about simply introducing a 'ampdu_mlme_lock_held' argument instead? Something like: diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c index 2b36eff5d97e..35a9eff1ec66 100644 --- a/net/mac80211/agg-rx.c +++ b/net/mac80211/agg-rx.c @@ -248,7 +248,8 @@ static void ieee80211_send_addba_resp(struct ieee80211_sub_if_data *sdata, u8 *d void __ieee80211_start_rx_ba_session(struct sta_info *sta, u8 dialog_token, u16 timeout, u16 start_seq_num, u16 ba_policy, u16 tid, - u16 buf_size, bool tx, bool auto_seq) + u16 buf_size, bool tx, bool auto_seq, + bool ampdu_mlme_lock_held) { struct ieee80211_local *local = sta->sdata->local; struct tid_ampdu_rx *tid_agg_rx; @@ -311,7 +312,8 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta, buf_size, sta->sta.addr); /* examine state machine */ - mutex_lock(&sta->ampdu_mlme.mtx); + if (!ampdu_mlme_lock_held) + mutex_lock(&sta->ampdu_mlme.mtx); if (test_bit(tid, sta->ampdu_mlme.agg_session_valid)) { if (sta->ampdu_mlme.tid_rx_token[tid] == dialog_token) { @@ -415,7 +417,8 @@ void __ieee80211_start_rx_ba_session(struct sta_info *sta, __clear_bit(tid, sta->ampdu_mlme.unexpected_agg); sta->ampdu_mlme.tid_rx_token[tid] = dialog_token; } - mutex_unlock(&sta->ampdu_mlme.mtx); + if (!ampdu_mlme_lock_held) + mutex_unlock(&sta->ampdu_mlme.mtx); end_no_lock: if (tx) @@ -445,7 +448,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, __ieee80211_start_rx_ba_session(sta, dialog_token, timeout, start_seq_num, ba_policy, tid, - buf_size, true, false); + buf_size, true, false, false); } void ieee80211_manage_rx_ba_offl(struct ieee80211_vif *vif, diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index c92df492e898..59ba67e8942f 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -335,7 +335,7 @@ void ieee80211_ba_session_work(struct work_struct *work) sta->ampdu_mlme.tid_rx_manage_offl)) __ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid, IEEE80211_MAX_AMPDU_BUF, - false, true); + false, true, true); if (test_and_clear_bit(tid + IEEE80211_NUM_TIDS, sta->ampdu_mlme.tid_rx_manage_offl)) diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 2197c62a0a6e..5d494ac65853 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1759,7 +1759,8 @@ void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid, void __ieee80211_start_rx_ba_session(struct sta_info *sta, u8 dialog_token, u16 timeout, u16 start_seq_num, u16 ba_policy, u16 tid, - u16 buf_size, bool tx, bool auto_seq); + u16 buf_size, bool tx, bool auto_seq, + bool ampdu_mlme_lock_held); void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta, enum ieee80211_agg_stop_reason reason); void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata,