From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x244.google.com (mail-lj1-x244.google.com [IPv6:2a00:1450:4864:20::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 5B8DF3B2A4 for ; Fri, 8 Nov 2019 20:04:21 -0500 (EST) Received: by mail-lj1-x244.google.com with SMTP id l21so727002ljg.4 for ; Fri, 08 Nov 2019 17:04:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Jt5B8kMSfET6dWwtqVV3OLABNUupSfHbIhe/rO2IyPc=; b=ZOds8rUQI6RmBvnjzyOCOAypkDjk6ncBrBAqUhqBAIk9Fom51HuvNZasIAEdV7g2Em LuNSBB1J7ArgchzeOcuh2fdTiTY8ZjL1jcP3LLiyviPIkz3mdFQ3u3zUgf94VGDlG1Yp CksIDR1HRG0IdDbJ115zxFW12o/a1X15x5CDtdRT0kqSY+miq8ThqsHC1BSNldQ5W/Vm EV88046zAj40k3W45Z0M+rglWL08bYAtTE66umd+jSrfftxFs5tE1f8lgJc/vQdIy+jN 4iVgcr+l+TMgjJt5JNP1HfgGxFkhbuO7Ym6GBjYV4tYUjx+KqFzbL0jcQWY2yczfIqI4 +f1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Jt5B8kMSfET6dWwtqVV3OLABNUupSfHbIhe/rO2IyPc=; b=dxWgyR3LpH+tpfhDtoIJXXHZ2b1ZPTLPUCcd4034o/iP9f5twkkqW2oaoWW3F1nz8b +IVcTk2zMA7GjJTYm1Z8HlN99KMUP3Vxw9lrAM48Co9KVadIdm7UbTcls10ScQyRi+5D N3JgGxXeuwTapjwjs8/Y2PEhmFRpUXq5fu7Mn3SkbLnoXpRYLHkCDApGN4XyBgOLLO2e h1qXMuZpWkDjb0F8Nxa+KwwuyB5yBBhRku4YB6RklssgpH/LWdyq3Umyk3WwtIivrKtA HgpfZ7jqIkNHBVvRGffWlySLPdVSZgSnO3tnv08Bo39FTsrjEXdaqAStOVVyKSkC1yqa Wqng== X-Gm-Message-State: APjAAAWvO0XrA9olG+Xr0/FCIzzo2v1baEvei1aPaoe82VpxdyHskaNv osJEr4Ye4BuvmNZUuhk/7qbOX/zAXQ4NrlyL5Qez9Q== X-Google-Smtp-Source: APXvYqyU+b04wCsnsuFN1qo5qVT1+ugNhWb2lRXbUXW1izTqJ7pBUwrgP99/NlpMF2TROSWoxEzHShuKnniJmirdbrE= X-Received: by 2002:a2e:92c4:: with SMTP id k4mr8616774ljh.10.1573261459891; Fri, 08 Nov 2019 17:04:19 -0800 (PST) MIME-Version: 1.0 References: <157182473951.150713.7978051149956899705.stgit@toke.dk> <157182474287.150713.12867638269538730397.stgit@toke.dk> <1a2eb096119c9029e67caf797564d6511c8803a7.camel@sipsolutions.net> <87a796fxgd.fsf@toke.dk> <874kzefwt3.fsf@toke.dk> <300bf0146db6c0d5890699b3911d35174d28c9c0.camel@sipsolutions.net> In-Reply-To: <300bf0146db6c0d5890699b3911d35174d28c9c0.camel@sipsolutions.net> From: Kan Yan Date: Fri, 8 Nov 2019 17:04:08 -0800 Message-ID: To: Johannes Berg Cc: =?UTF-8?B?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2Vu?= , linux-wireless@vger.kernel.org, Make-Wifi-fast , ath10k@lists.infradead.org, John Crispin , Lorenzo Bianconi , Felix Fietkau , Rajkumar Manoharan , Kevin Hayes Content-Type: multipart/alternative; boundary="00000000000074f9b90596df7aed" Subject: Re: [Make-wifi-fast] [PATCH v6 3/4] mac80211: Implement Airtime-based Queue Limit (AQL) X-BeenThere: make-wifi-fast@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Nov 2019 01:04:21 -0000 --00000000000074f9b90596df7aed Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable It is most likely just insufficient locking. active_txq_lock is per AC, can't protect local->aql_total_pending_airtime against racing conditions On Fri, Nov 8, 2019 at 3:17 AM Johannes Berg wrote: > On Fri, 2019-11-08 at 12:10 +0100, Toke H=C3=B8iland-J=C3=B8rgensen wrote= : > > > Right, bugger. I was thinking maybe there's a case where skbs can be > > cloned (and retain the tx_time_est field) and then released twice? > > They could be cloned, but I don't see how that'd be while *inside* the > stack and then they get reported twice - unless the driver did something > like that? > > I mean, TCP surely does that for example, but it's before we even get to > mac80211. > > > Or > > maybe somewhere that steps on the skb->cb field in some other way? > > Couldn't find anything obvious on a first perusal of the TX path code, > > but maybe you could think of something? > > No, sorry. But I also didn't actually look at the driver at all. > > > Otherwise I guess we'll be forced to go and do some actual, > > old-fashioned debugging ;) > > :) > > johannes > > --00000000000074f9b90596df7aed Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
It is most likely just insufficient=C2=A0locking.=C2=A0act= ive_txq_lock is per AC, can't protect local->aql_total_pending_airti= me against racing conditions


On Fri, Nov 8, 2019 at 3:17 AM J= ohannes Berg <johannes@sips= olutions.net> wrote:
On Fri, 2019-11-08 at 12:10 +0100, Toke H=C3=B8iland-J=C3=B8rge= nsen wrote:

> Right, bugger. I was thinking maybe there's a case where skbs can = be
> cloned (and retain the tx_time_est field) and then released twice?
They could be cloned, but I don't see how that'd be while *inside* = the
stack and then they get reported twice - unless the driver did something like that?

I mean, TCP surely does that for example, but it's before we even get t= o
mac80211.

> Or
> maybe somewhere that steps on the skb->cb field in some other way?<= br> > Couldn't find anything obvious on a first perusal of the TX path c= ode,
> but maybe you could think of something?

No, sorry. But I also didn't actually look at the driver at all.

> Otherwise I guess we'll be forced to go and do some actual,
> old-fashioned debugging ;)

:)

johannes

--00000000000074f9b90596df7aed--