From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h0cqQ-0001s4-A2 for ath10k@lists.infradead.org; Mon, 04 Mar 2019 01:56:43 +0000 MIME-Version: 1.0 Date: Mon, 04 Mar 2019 09:56:40 +0800 From: Yibo Zhao Subject: Re: FW: [PATCH] ath10k: fix return value check in wake_tx_q op In-Reply-To: References: <20180506132500.16888-1-erik.stromdahl@gmail.com> <6dc00772b826410e930306891fd13ed9@euamsexm01f.eu.qualcomm.com> <66a74025a23795f305de37989c1b8aa3@codeaurora.org> <87sgwz8ylw.fsf@kamboji.qca.qualcomm.com> Message-ID: <4dadbeebe8dc1e911cc4871d767b4ed1@codeaurora.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: base64 Content-Type: text/plain; charset="utf-8"; Format="flowed" Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Kalle Valo Cc: erik.stromdahl@gmail.com, linux-wireless-owner@vger.kernel.org, linux-wireless@vger.kernel.org, ath10k@lists.infradead.org 5ZyoIDIwMTktMDItMjUgMTI6NDDvvIxZaWJvIFpoYW8g5YaZ6YGT77yaCj4g5ZyoIDIwMTktMDIt MDcgMjI6MjXvvIxLYWxsZSBWYWxvIOWGmemBk++8mgo+PiBZaWJvIFpoYW8gPHlpYm96QGNvZGVh dXJvcmEub3JnPiB3cml0ZXM6Cj4+IAo+Pj4gV2UgaGF2ZSBtZXQgcGVyZm9ybWFuY2UgaXNzdWUg b24gb3VyIHR3by1jb3JlIHN5c3RlbSBhZnRlciBhcHBseWluZwo+Pj4geW91ciBwYXRjaC4gSW4g V0RTIG1vZGUsIHdlIGZvdW5kIHRoYXQgdGhlIHBlYWsgdGhyb3VnaHB1dCBpbiBUQ1AtREwKPj4+ IGFuZCBVRFAtREwgZHJvcHBlZCBtb3JlIHRoYW4gMTAlIGNvbXBhcmVkIHdpdGggcHJldmlvdXMg b25lLiBBbmQgaW4KPj4+IHNvbWUgY2FzZXMsIHRob3VnaCB0aHJvdWdocHV0IHN0YXlzIHRoZSBz YW1lLCBvbmUgQ1BVIHVzYWdlIHJpc2VzCj4+PiBhYm91dCAyMCUgd2hpY2ggbGVhZHMgdG8gMTAl IGluIHRvdGFsIENQVSB1c2FnZS4gV2l0aCB5b3VyIGNoYW5nZSwgSQo+Pj4gdGhpbmsgZHJpdmVy IHdpbGwgdHJ5IGl0cyBiZXN0IHRvIHB1c2ggYXMgbWFueSBwYWNrZXRzIGFzIGl0IGNhbi4KPj4+ IER1cmluZyB0aGlzIHRpbWUsIHRoZSBkcml2ZXIncyBxdWV1ZSBsb2NrIHdpbGwgYmUgaGVsZCBm b3IgdG9vIG11Y2gKPj4+IHRpbWUgaW4gb25lIENQVSBhbmQgYXMgYSByZXN1bHQsIHRoZSBvdGhl ciBDUFUgd2lsbCBiZSBibG9ja2VkIGlmIGl0Cj4+PiB3YW50cyB0byBhY3F1aXJlZCB0aGUgc2Ft ZSBsb2NrLiBXb3JraW5nIGluIHRoaXMgd2F5IHNlZW1zIG5vdAo+Pj4gZWZmaWNpZW5jeS4KPj4+ IAo+Pj4gU28gSSB0aGluayBpdCBpcyBiZXR0ZXIgdG8gcmV2ZXJ0IHRoZSBjaGFuZ2UgdGlsbCB3 ZSBjb21lIHVwIHdpdGggYQo+Pj4gbmV3IHNvbHV0aW9uLgo+PiAKPj4gSSBkb24ndCB0aGluayBy ZXZlcnRpbmcgaXMgYSBjbGVhciBvcHRpb24gYXQgdGhpcyBzdGFnZSBiZWNhdXNlIHRoYXQKPj4g YWdhaW4gY3JlYXRlcyBwcm9ibGVtcyBmb3IgU0RJTy4gSUlSQyB3aXRob3V0IHRoaXMgcGF0Y2gg U0RJTyB3YXMKPj4gc2VuZGluZyBvbmUgcGFja2V0IGEgdGltZSAob3Igc29tZXRoaW5nIGxpa2Ug dGhhdCwgY2FuJ3QgcmVtZW1iZXIgYWxsCj4+IHRoZSBkZXRhaWxzIHJpZ2h0IG5vdykuCj4+IAo+ IAo+IEJlbG93IGlzIHRoZSBhcW0gcmVzdWx0IGFmdGVyIDEgbWluIHRlc3Qgd2l0aCBFcmlrJ3Mg cGF0Y2guCj4gCj4gdGFyZ2V0IDE5OTk5dXMgaW50ZXJ2YWwgOTk5OTl1cyBlY24geWVzCj4gdGlk IGFjIGJhY2tsb2ctYnl0ZXMgYmFja2xvZy1wYWNrZXRzIG5ldy1mbG93cyBkcm9wcyBtYXJrcyBv dmVybGltaXQKPiBjb2xsaXNpb25zIHR4LWJ5dGVzIHR4LXBhY2tldHMgZmxhZ3MKPiAwIDIgMCAw IDUzNDIyMjkgMCAwIDAgMCAzODY3NjU3MDI5IDUzNDIyMjkgMHgwKFJVTikKPiAxIDMgMCAwIDAg MCAwIDAgMCAwIDAgMHgwKFJVTikKPiAyIDMgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiAz IDIgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiA0IDEgMCAwIDAgMCAwIDAgMCAwIDAgMHgw KFJVTikKPiA1IDEgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiA2IDAgMCAwIDIgMCAwIDAg MCAxNDQgMiAweDAoUlVOKQo+IDcgMCAwIDAgMiAwIDAgMCAwIDI4MiAyIDB4MChSVU4pCj4gOCAy IDAgMCAwIDAgMCAwIDAgMCAwIDB4MChSVU4pCj4gOSAzIDAgMCAwIDAgMCAwIDAgMCAwIDB4MChS VU4pCj4gMTAgMyAwIDAgMCAwIDAgMCAwIDAgMCAweDAoUlVOKQo+IDExIDIgMCAwIDAgMCAwIDAg MCAwIDAgMHgwKFJVTikKPiAxMiAxIDAgMCAwIDAgMCAwIDAgMCAwIDB4MChSVU4pCj4gMTMgMSAw IDAgMCAwIDAgMCAwIDAgMCAweDAoUlVOKQo+IDE0IDAgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJV TikKPiAxNSAwIDAgMCAwIDAgMCAwIDAgMCAwIDB4MChSVU4pCj4gCj4gd2Ugc2VlIG5vIGRpZmZl cmVuY2UgYmV0d2VlbiB0eC1wYWNrZXRzIGFuZCBuZXctZmxvd3MuCj4gV2hlcmVhcyBpZiB3ZSBy ZXZlcnQgdGhlIHBhdGNoLCB3ZSBnZXTvvJoKPiAKPiB0YXJnZXQgMTk5OTl1cyBpbnRlcnZhbCA5 OTk5OXVzIGVjbiB5ZXMKPiB0aWQgYWMgYmFja2xvZy1ieXRlcyBiYWNrbG9nLXBhY2tldHMgbmV3 LWZsb3dzIGRyb3BzIG1hcmtzIG92ZXJsaW1pdAo+IGNvbGxpc2lvbnMgdHgtYnl0ZXMgdHgtcGFj a2V0cyBmbGFncwo+IDAgMiAwIDAgMjIzMzA1OSAzIDAgOTIzNiAxMiAxMTU5NjYxNzgzIDYzODA4 NjcgMHgwKFJVTikKPiAxIDMgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiAyIDMgMCAwIDAg MCAwIDAgMCAwIDAgMHgwKFJVTikKPiAzIDIgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiA0 IDEgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiA1IDEgMCAwIDAgMCAwIDAgMCAwIDAgMHgw KFJVTikKPiA2IDAgMCAwIDEgMCAwIDAgMCAxNDQgMiAweDAoUlVOKQo+IDcgMCAwIDAgMSAwIDAg MCAwIDI4MiAyIDB4MChSVU4pCj4gOCAyIDAgMCAwIDAgMCAwIDAgMCAwIDB4MChSVU4pCj4gOSAz IDAgMCAwIDAgMCAwIDAgMCAwIDB4MChSVU4pCj4gMTAgMyAwIDAgMCAwIDAgMCAwIDAgMCAweDAo UlVOKQo+IDExIDIgMCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiAxMiAxIDAgMCAwIDAgMCAw IDAgMCAwIDB4MChSVU4pCj4gMTMgMSAwIDAgMCAwIDAgMCAwIDAgMCAweDAoUlVOKQo+IDE0IDAg MCAwIDAgMCAwIDAgMCAwIDAgMHgwKFJVTikKPiAxNSAwIDAgMCAwIDAgMCAwIDAgMCAwIDB4MChS VU4pCj4gCj4gbmV3LWZsb3dzIGFyZSByb3VnaGx5IG9uZS10aGlyZCBvZiB0aGUgdG90YWwgdHgt cGFja2V0cy4KPiAKPiBJTUhPLCB3aXRoIEVyaWsncyBjaGFuZ2UsIEVyaWsncyBjaGFuZ2UgaGFz IGNoYW5nZWQgdGhlIHdheSBmcSdzCj4gc2NoZWR1bGUgYmVoYXZpb3IgYW5kIGl0IGxvb2tzIGxp a2UgdGhlcmUgaXMgbm8gb3RoZXIgcGFja2V0cyBpbiB0aGUKPiBmcSBhZnRlciBhIHBhY2tldCBo YXMgYmVlbiBkZXF1ZXVlZC4gQW5kIGFzIGEgcmVzdWx0LCB0aGlzIGZsb3cncwo+IGRlZmljaXQg d2lsbCBiZSByZWZpbGwgYW5kIHRoZW4gcmVtb3ZlZCBmcm9tIGZxIGxpc3QgYXQgb25jZSBpbiB0 aGUKPiBzYW1lIENQVS4gQW5kIGR1cmluZyB0aGlzIHRpbWUsIHRoZSBvdGhlciBDUFUgY291bGQg YmUgYmxvY2tlZC4gV2hlbgo+IG5ldyBwYWNrZXQgY29tZXMsIHNhbWUgdGhpbmcgaGFwcGVucy4g U28gd2UgZ2V0IGVxdWFsIG5ldyBmbG93cyBhbmQKPiB0eC1wYWNrZXRzLgo+IAo+IFRoaW5ncyB3 b3VsZCBiZSBkaWZmZXJlbnQgd2l0aG91dCBFcmlrJ3MgY2hhbmdlLiBBZnRlciBhIHBhY2tldCBo YXMKPiBiZWVuIGRlcXVldWVkLCB0aGlzIGZsb3cncyBkZWZpY2l0IHdpbGwgbm90IGJlIHJlZmls bCBpbW1lZGlhdGVseSBpbgo+IENQVTAuIEl0IGlzIHBvc3NpYmxlIHRoYXQgdGhlIGRlZmljaXQg dG8gYmUgcmVmaWxsZWQgaW4gQ1BVMSB3aGlsZSBhdAo+IHRoZSBzYW1lIHRpbWUgQ1BVMCBjYW4g ZmV0Y2ggZGF0YSBmcm9tIGV0aGVybmV0LiBTbyB3ZSBjYW4gc2VlIG1vcmUKPiB0eC1wYWNrZXRz IGRlbGl2ZXJlZCB0byBGVyBmcm9tIGFxbS4KPiAKPiAKPj4gV2h5IGRvZXMgdGhpcyBoYXBwZW4g b25seSBXRFMgbW9kZT8gRGlkIHlvdSB0ZXN0IG90aGVyIG1vZGVzLCBsaWtlIEFQIAo+PiBvcgo+ PiBjbGllbnQgbW9kZT8KPiBBUCBtb2RlIGhhcyBzYW1lIGlzc3VlLiBVRFAgdGhyb3VnaHB1dCBk cm9wcyBtb3JlIHRoYW4gMTAlLiBBcyBmb3IKPiBUQ1AsIENQVSB1c2FnZSByaXNpbmcgYSBsb3Qg YWx0aG91Z2ggdGhyb3VnaHB1dCBzdGF5cyBzaW1pbGlhci4KPiBTbywgSSdkIHNheSBFcmlrJ3Mg Y2hhbmdlIGRvZXMgbm90IHdvcmsgZm9yIHVzLgoKSGkgS2FsbGUsCgpNYXkgSSBoYXZlIHlvdXIg Y29tbWVudHM/CgotLSAKWWlibwoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KYXRoMTBrIG1haWxpbmcgbGlzdAphdGgxMGtAbGlzdHMuaW5mcmFkZWFkLm9y ZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2F0aDEwawo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4166AC43381 for ; Mon, 4 Mar 2019 01:56:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0E89D20835 for ; Mon, 4 Mar 2019 01:56:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="jRfQuXg4"; dkim=fail reason="key not found in DNS" (0-bit key) header.d=codeaurora.org header.i=@codeaurora.org header.b="mENzBKWU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726137AbfCDB4n (ORCPT ); Sun, 3 Mar 2019 20:56:43 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:44020 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726012AbfCDB4m (ORCPT ); Sun, 3 Mar 2019 20:56:42 -0500 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 4A5EB601FE; Mon, 4 Mar 2019 01:56:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551664601; bh=BQAjh8vr0NrWhgNZRrWmgdTYeD0kWc5QrI2K6+oZq24=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=jRfQuXg40bEcouhEehox2XjXhJ8c9UBJRIajWr9acrGijEn262DFoGXjwWcb0c4ZW 4fxQr9rVcRsAWsrmsDGAtwPm3FM8wwDToGlbOXMk8wv55CEa6nbETuEKXef0qANBQ7 WwNMw78d7jWgy0BWqMFXaenofHD/Fgp5puKfYJ3I= Received: from mail.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 54E19601FE; Mon, 4 Mar 2019 01:56:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1551664600; bh=BQAjh8vr0NrWhgNZRrWmgdTYeD0kWc5QrI2K6+oZq24=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=mENzBKWU2UNR91ugIeF6oi+TISNo3Jm1p8qb0yYYfBN56y02Yw6QS+UD3i9GLY8V6 9yKy4CGcCrlMIEXj10+MyKS7v6AFovmMxrzb4dkvT5N8xTbwy2wLONFDFr6DWdEN6r 99PodYa/WahpKvUnIKfSKOKqJ4QcvsCG478AaE8E= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Mon, 04 Mar 2019 09:56:40 +0800 From: Yibo Zhao To: Kalle Valo Cc: erik.stromdahl@gmail.com, linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, linux-wireless-owner@vger.kernel.org Subject: Re: FW: [PATCH] ath10k: fix return value check in wake_tx_q op In-Reply-To: References: <20180506132500.16888-1-erik.stromdahl@gmail.com> <6dc00772b826410e930306891fd13ed9@euamsexm01f.eu.qualcomm.com> <66a74025a23795f305de37989c1b8aa3@codeaurora.org> <87sgwz8ylw.fsf@kamboji.qca.qualcomm.com> Message-ID: <4dadbeebe8dc1e911cc4871d767b4ed1@codeaurora.org> X-Sender: yiboz@codeaurora.org User-Agent: Roundcube Webmail/1.2.5 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org 在 2019-02-25 12:40，Yibo Zhao 写道： > 在 2019-02-07 22:25，Kalle Valo 写道： >> Yibo Zhao writes: >> >>> We have met performance issue on our two-core system after applying >>> your patch. In WDS mode, we found that the peak throughput in TCP-DL >>> and UDP-DL dropped more than 10% compared with previous one. And in >>> some cases, though throughput stays the same, one CPU usage rises >>> about 20% which leads to 10% in total CPU usage. With your change, I >>> think driver will try its best to push as many packets as it can. >>> During this time, the driver's queue lock will be held for too much >>> time in one CPU and as a result, the other CPU will be blocked if it >>> wants to acquired the same lock. Working in this way seems not >>> efficiency. >>> >>> So I think it is better to revert the change till we come up with a >>> new solution. >> >> I don't think reverting is a clear option at this stage because that >> again creates problems for SDIO. IIRC without this patch SDIO was >> sending one packet a time (or something like that, can't remember all >> the details right now). >> > > Below is the aqm result after 1 min test with Erik's patch. > > target 19999us interval 99999us ecn yes > tid ac backlog-bytes backlog-packets new-flows drops marks overlimit > collisions tx-bytes tx-packets flags > 0 2 0 0 5342229 0 0 0 0 3867657029 5342229 0x0(RUN) > 1 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 2 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 3 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 4 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 5 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 6 0 0 0 2 0 0 0 0 144 2 0x0(RUN) > 7 0 0 0 2 0 0 0 0 282 2 0x0(RUN) > 8 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 9 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 10 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 11 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 12 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 13 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 14 0 0 0 0 0 0 0 0 0 0 0x0(RUN) > 15 0 0 0 0 0 0 0 0 0 0 0x0(RUN) > > we see no difference between tx-packets and new-flows. > Whereas if we revert the patch, we get： > > target 19999us interval 99999us ecn yes > tid ac backlog-bytes backlog-packets new-flows drops marks overlimit > collisions tx-bytes tx-packets flags > 0 2 0 0 2233059 3 0 9236 12 1159661783 6380867 0x0(RUN) > 1 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 2 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 3 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 4 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 5 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 6 0 0 0 1 0 0 0 0 144 2 0x0(RUN) > 7 0 0 0 1 0 0 0 0 282 2 0x0(RUN) > 8 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 9 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 10 3 0 0 0 0 0 0 0 0 0 0x0(RUN) > 11 2 0 0 0 0 0 0 0 0 0 0x0(RUN) > 12 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 13 1 0 0 0 0 0 0 0 0 0 0x0(RUN) > 14 0 0 0 0 0 0 0 0 0 0 0x0(RUN) > 15 0 0 0 0 0 0 0 0 0 0 0x0(RUN) > > new-flows are roughly one-third of the total tx-packets. > > IMHO, with Erik's change, Erik's change has changed the way fq's > schedule behavior and it looks like there is no other packets in the > fq after a packet has been dequeued. And as a result, this flow's > deficit will be refill and then removed from fq list at once in the > same CPU. And during this time, the other CPU could be blocked. When > new packet comes, same thing happens. So we get equal new flows and > tx-packets. > > Things would be different without Erik's change. After a packet has > been dequeued, this flow's deficit will not be refill immediately in > CPU0. It is possible that the deficit to be refilled in CPU1 while at > the same time CPU0 can fetch data from ethernet. So we can see more > tx-packets delivered to FW from aqm. > > >> Why does this happen only WDS mode? Did you test other modes, like AP >> or >> client mode? > AP mode has same issue. UDP throughput drops more than 10%. As for > TCP, CPU usage rising a lot although throughput stays similiar. > So, I'd say Erik's change does not work for us. Hi Kalle, May I have your comments? -- Yibo