From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 1539C3B29D for ; Fri, 15 Nov 2019 07:56:11 -0500 (EST) 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-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-258-dAF7uHxZNpKujhnYejwFNg-1; Fri, 15 Nov 2019 07:56:07 -0500 Received: by mail-lf1-f70.google.com with SMTP id v13so3054957lfq.2 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=poGMREXYwN87lQjtlAWP1IGVJcm77yUEOTIxh9sGjULcKFbb7flW5w2SzHJVvVaKPu K5j0gqkLekiS6Ubs8UZCYzSM5fni6yRbp+sxHBfkhqemL33uw0MfcyQbuEDAT2qtq2MZ w5mCYD1GZ66ItKbuJXzHf4HxeOFzClRO5BYshu3FrItANRZCxWWa4jJxlPFlsNLl+Bid PUwAFMIike9s/DM2iAJJ9xSACb/l9JT78xUeqAqy1cuZCB5unEMbbj7nmeSd9V0gcOvu oeSKN0Fif8jgFgQUjO37XnUS7XJB2xsFD2cpukSWGK5Zkp7EilD0L0skLlOilc7+5gzP Ek5Q== X-Gm-Message-State: APjAAAW+QgPHX/rI1qYQn3uUw1LSrIoA7bGLbBbsXRRitdf3hV4FItMs OcgDpqSTVJK1tA0ilsLe9QVX/rkF8sn2+DbtWnbQ7h5e9DXcqN6BsshdYo5i3OMMfcNJUBSphaw q19h2t9u1aTdWRo03K4hLcgcDXJJNS9W6QCU= X-Received: by 2002:a2e:9097:: with SMTP id l23mr11205505ljg.103.1573822565816; Fri, 15 Nov 2019 04:56:05 -0800 (PST) X-Google-Smtp-Source: APXvYqz7I67ctwzPxNDc7tNLEE71a8hkkTT8O7kW3O9+UMjFqJcaX8XCSvR/UcDmLGKgz+qm81NA5w== 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 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: dAF7uHxZNpKujhnYejwFNg-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Make-wifi-fast] [v8 PATCH 2/2] 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: Fri, 15 Nov 2019 12:56:11 -0000 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