Received: by 2002:a25:7ec1:0:0:0:0:0 with SMTP id z184csp4449351ybc; Fri, 15 Nov 2019 04:59:33 -0800 (PST) X-Google-Smtp-Source: APXvYqxt5z/OuXsAh1525VguAf//8gacbG+WGH5QeZHqGFSZO0+ORZ1c0vi91xCg+23AmaZUmqcj X-Received: by 2002:a17:906:1d19:: with SMTP id n25mr703589ejh.151.1573822773231; Fri, 15 Nov 2019 04:59:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1573822773; cv=none; d=google.com; s=arc-20160816; b=KtCb7Mx9rPBG5CLL6dHaOWhSQxB/oOMjRatS6/grjuUgk+Fr4fzcfrnvmpYbtjh+HP uDUOjurbwhItQYZGwE3Fzl4NRWg00piRoWcXtu02grQ4VkhHeDZerbTqv9VRAWP2abRS wmlz0R7tG1SbabWOirzLRE3bmlLSRs52dGRKirPUR44jbL1wptxyUvs9odM9y7tN+cum klX6vBrjGXfj/Sy2bf/jY9qlsBlwirTGxuVfx1ebgV+r6KeTG3qU63hNBGwVQpzv9BQ7 QzfF+usu6N5b17OCTAfFrHL333zuwUOj7oPltoz+/jI7Rm9GHMl1h5dciB0PYoqwqC8W pxdw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :message-id:date:references:in-reply-to:subject:cc:to:from :dkim-signature; bh=95KCD774oTBhtPAun5LFxt7PQ5lOqiqNZiBq+HVC7WI=; b=haCHeSZhFo+iVD3+YLlaO0g865la/vi2lAjWZBNDz3voElFNtYmW4YYe7bfl/omgMS W4R5IxhYDq36R7xOHVJ9CeEnhBUjT2D6i+d9eIo+hK4pAT/TVxynD847q/ekkMYerxTn tOrCCQJe3iPQoOc2eRB+8WZCfmdM5uIWbwDVE+TmdS4U11qXan6PSfITCcrlgv31Dwi0 Nql+zRxHqE6v+1rW1Szqy3SFZ+l2PMe4Uhsz+aMM+VsjkyFwxyHXC8Xjqs0D3B+yL4ao PncSH0Z0hPxoRfz4yhxcAl7B/jL35H8H0Dbu69+Zw7nl6xQs+HTg+L4Ohh9EWJqJz6u1 mLoA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RmzeKqDp; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id d24si6533168eda.162.2019.11.15.04.58.57; Fri, 15 Nov 2019 04:59:33 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RmzeKqDp; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727329AbfKOM4L (ORCPT + 99 others); Fri, 15 Nov 2019 07:56:11 -0500 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:37388 "EHLO us-smtp-1.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727223AbfKOM4L (ORCPT ); Fri, 15 Nov 2019 07:56:11 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573822570; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=95KCD774oTBhtPAun5LFxt7PQ5lOqiqNZiBq+HVC7WI=; b=RmzeKqDpL6ZLTRVqEqB2Mcu4DCufqG6DIQQdyyOlxsNohPG2V/mFTD/TLbv9gkAueJUydf br3NrwpsMl2pplwh6+IhQ9USqGql+DHH5OMoRkiZJAOUuD5hBNLHgXEaFuP5z0qHBgCR7u RwGZTE3ckuJDQ7WBFRz7omrD72+0N5Q= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-19-KAcZdrXPMpugYrCYwas3Jw-1; Fri, 15 Nov 2019 07:56:07 -0500 Received: by mail-lf1-f72.google.com with SMTP id t6so3044125lfd.13 for ; Fri, 15 Nov 2019 04:56:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=6a+NU/C31nl6e5FCe8WL5WTU7WvA118zrjXjtwzD8E0=; b=uXjr9fA/dSZdeAsREaCBWmaJKgQ6IFUvO9Yfs9nFP8VMPSEqBG402vNt3878SBRuAC pijy+EmLsqz/WuwV5DQbLLybc2+BTCQbC7OS51Tu0uyrl3KCc5fEUabpqV6qSXPVsFGT anAF9ocoy+T2AhYnoCfyT3AJ1U3AH40Qgkb63ZcNKXouhHUwg4apKSvP/o/CnJsdgHjs luN2g3hmXY/0ytYtfxkEVm4pxlFh1Yjh8kxEONEAeXqbJluK7AAVOmUA+YbFaSliisD5 mzAJQt0cQ+xt11RiiSmnWNocUN49mB/yisQ2I+gbwzFrZE56g5HELkDpuVaLre/fv34D UN5g== X-Gm-Message-State: APjAAAW98KuezpuTbVZyNb3j2sxA8vGV3zc1ynzMaNg6FSApjMyqAacW Pi1+Kw5gbDZ/kA/ZOBfWsK2JfyZVsRiqk5+B7fS1QmTI3FbDtLI7lW9/QNn6rUx3zP0R3irOTH0 5sZJMPzcfQIGv3YTZkQ7eJZ6QygE= X-Received: by 2002:a2e:9097:: with SMTP id l23mr11205507ljg.103.1573822565816; Fri, 15 Nov 2019 04:56:05 -0800 (PST) X-Received: by 2002:a2e:9097:: with SMTP id l23mr11205487ljg.103.1573822565564; Fri, 15 Nov 2019 04:56:05 -0800 (PST) Received: from alrua-x1.borgediget.toke.dk (borgediget.toke.dk. [85.204.121.218]) by smtp.gmail.com with ESMTPSA id q11sm3870857ljm.107.2019.11.15.04.56.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Nov 2019 04:56:04 -0800 (PST) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id 1C92B1818C5; Fri, 15 Nov 2019 13:56:04 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Kan Yan , johannes@sipsolutions.net Cc: linux-wireless@vger.kernel.org, make-wifi-fast@lists.bufferbloat.net, nbd@nbd.name, yiboz@codeaurora.org, john@phrozen.org, lorenzo@kernel.org, rmanohar@codeaurora.org, kevinhayes@google.com, Kan Yan Subject: Re: [v8 PATCH 2/2] mac80211: Implement Airtime-based Queue Limit (AQL) In-Reply-To: <20191115014846.126007-3-kyan@google.com> References: <20191115014846.126007-1-kyan@google.com> <20191115014846.126007-3-kyan@google.com> X-Clacks-Overhead: GNU Terry Pratchett Date: Fri, 15 Nov 2019 13:56:04 +0100 Message-ID: <87k181mh7f.fsf@toke.dk> MIME-Version: 1.0 X-MC-Unique: KAcZdrXPMpugYrCYwas3Jw-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org Kan Yan writes: > In order for the Fq_CoDel algorithm integrated in mac80211 layer to opera= te > effectively to control excessive queueing latency, the CoDel algorithm > requires an accurate measure of how long packets stays in the queue, AKA > sojourn time. The sojourn time measured at the mac80211 layer doesn't > include queueing latency in the lower layer (firmware/hardware) and CoDel > expects lower layer to have a short queue. However, most 802.11ac chipset= s > offload tasks such TX aggregation to firmware or hardware, thus have a de= ep > lower layer queue. > > Without a mechanism to control the lower layer queue size, packets only > stay in mac80211 layer transiently before being sent to firmware queue. > As a result, the sojourn time measured by CoDel in the mac80211 layer is > almost always lower than the CoDel latency target, hence CoDel does littl= e > to control the latency, even when the lower layer queue causes excessive > latency. > > The Byte Queue Limits (BQL) mechanism is commonly used to address the > similar issue with wired network interface. However, this method cannot b= e > applied directly to the wireless network interface. "Bytes" is not a > suitable measure of queue depth in the wireless network, as the data rate > can vary dramatically from station to station in the same network, from a > few Mbps to over Gbps. > > This patch implements an Airtime-based Queue Limit (AQL) to make CoDel wo= rk > effectively with wireless drivers that utilized firmware/hardware > offloading. AQL allows each txq to release just enough packets to the low= er > layer to form 1-2 large aggregations to keep hardware fully utilized and > retains the rest of the frames in mac80211 layer to be controlled by the > CoDel algorithm. > > Signed-off-by: Kan Yan > [ Toke: Keep API to set pending airtime internal, fix nits in commit msg = ] > Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen > --- [...] > +=09if (sta) { > +=09=09atomic_sub(tx_airtime, &sta->airtime[ac].aql_tx_pending); > +=09=09tx_pending =3D atomic_read(&sta->airtime[ac].aql_tx_pending); This is still racy, since you're splitting it over two calls; you'll need to use atomic_sub_return() instead. I figure we've converged now to the point where it actually makes sense to collect everything back into a single series; so I can just fix this and re-send the full series. > +=09=09if (WARN_ONCE(tx_pending < 0, > +=09=09=09 "STA %pM AC %d txq pending airtime underflow: %u, %u", > +=09=09=09 sta->addr, ac, tx_pending, tx_airtime)) > +=09=09=09atomic_cmpxchg(&sta->airtime[ac].aql_tx_pending, > +=09=09=09=09 tx_pending, 0); This could still fail if there's a concurrent modification (and you're not checking the return of the cmpxchg). But at least it won't clobber any updated value, so I guess that is acceptable since we're in "should never happen" territory here :) -Toke