From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <toke@toke.dk>
Received: from mail.toke.dk (mail.toke.dk [52.28.52.200])
 (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by lists.bufferbloat.net (Postfix) with ESMTPS id CA55E3B29E
 for <make-wifi-fast@lists.bufferbloat.net>;
 Fri, 21 Sep 2018 08:41:17 -0400 (EDT)
From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023;
 t=1537533676; bh=gCCpn/iVylIpQmIoVspKOdSmpxL3MsZ6eHbdEySw7Ro=;
 h=From:To:Cc:Subject:In-Reply-To:References:Date:From;
 b=Y9ciaGqBF0lC6kEiZlPnNVtbL4ywcYMKWOnpYt1NuuluwOTtkNArfDuV5rEMlykDc
 D0e64dzDhkzago8X/9ZPH8QA4vTr2YAv7Glu4i3yaGGUOnEEP7ISgQJrew/IBBf1S4
 cK0e00wfa70jj+keWHRABiwudBPO3RGXvRgrdwFGazuvHGz7PU5RUqiJxSS0+qdmxB
 rn/mhwda2/IYsGFmbcP1JYw5e4ln14+H/726XEGOzIaae4yY881thiDz3mieLIuPIQ
 gcQMzROykmLnimAh02XjXTUNHTFxEGivZPK8dMjLgtMCPGRSq1CtxNnZUD/icUVI3s
 jjWm17Hlw/q0Q==
To: Rajkumar Manoharan <rmanohar@codeaurora.org>
Cc: linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net,
 Felix Fietkau <nbd@nbd.name>, Kan Yan <kyan@google.com>
In-Reply-To: <6f97d6300d514c0b6577c9c9376b98a8@codeaurora.org>
References: <153711966150.9231.13481453399723518107.stgit@alrua-x1>
 <6f97d6300d514c0b6577c9c9376b98a8@codeaurora.org>
Date: Fri, 21 Sep 2018 14:41:15 +0200
X-Clacks-Overhead: GNU Terry Pratchett
Message-ID: <87tvmjhvkk.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Make-wifi-fast] [PATCH RFC v4 0/4] Move TXQ scheduling into
	mac80211
X-BeenThere: make-wifi-fast@lists.bufferbloat.net
X-Mailman-Version: 2.1.20
Precedence: list
List-Id: <make-wifi-fast.lists.bufferbloat.net>
List-Unsubscribe: <https://lists.bufferbloat.net/options/make-wifi-fast>,
 <mailto:make-wifi-fast-request@lists.bufferbloat.net?subject=unsubscribe>
List-Archive: <https://lists.bufferbloat.net/pipermail/make-wifi-fast>
List-Post: <mailto:make-wifi-fast@lists.bufferbloat.net>
List-Help: <mailto:make-wifi-fast-request@lists.bufferbloat.net?subject=help>
List-Subscribe: <https://lists.bufferbloat.net/listinfo/make-wifi-fast>,
 <mailto:make-wifi-fast-request@lists.bufferbloat.net?subject=subscribe>
X-List-Received-Date: Fri, 21 Sep 2018 12:41:18 -0000

Rajkumar Manoharan <rmanohar@codeaurora.org> writes:

> On 2018-09-16 10:42, Toke H=C3=B8iland-J=C3=B8rgensen wrote:
>> Another update, addressing most of the concerns raised in the last=20
>> round:
>>=20
>> - Added schedule_start()/end() functions that adds locking around the
>>   whole scheduling operation, which means we can get rid of the 'first'
>>   parameter to ieee80211_next_txq().
>>=20
> Toke,
>
> Wouldn't this start()/end() APIs cause deadlock if mac80211 tries to=20
> acquire
> active_txq_lock[ac] again? Or am I missing?
>
> schedule_start()
> while (next_txq()) {
>    push_txq -> tx_dequeue()
>    return_txq()
> }
> schedule_end()
>
> tx_dequeue()
>   ieee80211_free_txskb
>    -> ieee80211_report_used_skb
>     -> ieee80211_tdls_td_tx_handle
>      -> ieee80211_subif_start_xmit
>      -> __ieee80211_subif_start_xmit
>        -> ieee80211_xmit_fast
>          -> ieee80211_queue_skb
>           -> schedule_and_wake_txq

Hmm, yeah, that call sequence would certainly deadlock. But earlier, I
think; ieee80211_queue_skb() already takes fq->lock, which is being held
by tx_dequeue(), so this would deadlock on that?

I guess retransmits of TDLS teardown packets don't happen so often?
Otherwise someone would have run into this by now? Or are those frames
not being transmitted through the TXQs at all?

In any case it does seem a bit dangerous to have a possible path where
ieee80211_free_txskb() will result in a call to
ieee80211_subif_start_xmit(). So we should probably fix that in any
case?

-Toke