From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.candelatech.com ([208.74.158.172] helo=ns3.lanforge.com) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WSp0M-0000bl-Fw for ath10k@lists.infradead.org; Wed, 26 Mar 2014 14:40:35 +0000 Message-ID: <5332E6C6.7040700@candelatech.com> Date: Wed, 26 Mar 2014 07:40:06 -0700 From: Ben Greear MIME-Version: 1.0 Subject: Re: [PATCH 1/3] ath10k: Add debugging for tx-credits usage. References: <1395428150-31996-1-git-send-email-greearb@candelatech.com> <871txr7svw.fsf@kamboji.qca.qualcomm.com> <53306239.2020006@candelatech.com> <53319D65.4090907@candelatech.com> In-Reply-To: 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: Michal Kazior Cc: Kalle Valo , linux-wireless , "ath10k@lists.infradead.org" T24gMDMvMjYvMjAxNCAxMjoxOCBBTSwgTWljaGFsIEthemlvciB3cm90ZToKPiBPbiAyNSBNYXJj aCAyMDE0IDE2OjE0LCBCZW4gR3JlZWFyIDxncmVlYXJiQGNhbmRlbGF0ZWNoLmNvbT4gd3JvdGU6 Cj4+IE9uIDAzLzI1LzIwMTQgMTI6MjcgQU0sIE1pY2hhbCBLYXppb3Igd3JvdGU6Cj4+Pgo+Pj4g T24gMjQgTWFyY2ggMjAxNCAxNzo1MCwgQmVuIEdyZWVhciA8Z3JlZWFyYkBjYW5kZWxhdGVjaC5j b20+IHdyb3RlOgo+Pj4+Cj4+Pj4gT24gMDMvMjQvMjAxNCAwNDoyMSBBTSwgTWljaGFsIEthemlv ciB3cm90ZToKPj4+Pj4KPj4+Pj4gT24gMjQgTWFyY2ggMjAxNCAxMjoxMiwgS2FsbGUgVmFsbyA8 a3ZhbG9AcWNhLnF1YWxjb21tLmNvbT4gd3JvdGU6Cj4+Pj4KPj4+Pgo+Pj4+Pj4+IC0gICAgIHN0 YXR1cyA9IGF0aDEwa19odGNfc2VuZChodGMsIEFUSDEwS19IVENfRVBfMCwgc2tiKTsKPj4+Pj4+ PiArICAgICBzdGF0dXMgPSBhdGgxMGtfaHRjX3NlbmQoaHRjLCBBVEgxMEtfSFRDX0VQXzAsIHNr YiwgX19MSU5FX18pOwo+Pj4+Pj4KPj4+Pj4+Cj4+Pj4+PiBVc2luZyBsaW5lIG51bWJlcnMgaW4g ZGVidWcgbWVzc2FnZXMgaXMgdmVyeSBjdW1iZXJzb21lLiBTb21lIHBlb3BsZQo+Pj4+Pj4gY2hl cnJ5IHBpY2sgcGF0Y2hlcywgaGF2ZSB0aGVpciBvd24gY2hhbmdlcyBhbmQgd2hhdG5vdCB3aGlj aCB3aWxsIG1ha2UKPj4+Pj4+IGl0IG1vcmUgZGlmZmljdWx0IHRvIHJlYWQgdGhlIGRlYnVnIGxv Z3MuIElzbid0IHRoZXJlIGFueSBiZXR0ZXIgd2F5IHRvCj4+Pj4+PiBkbyB0aGlzPwo+Pj4+Pgo+ Pj4+Pgo+Pj4+PiBJIHdvdWxkIHByZWZlciB0byBleHBsaWNpdGx5IHdhaXQgZm9yIHR4IGNyZWRp dCByZXBsZW5pc2htZW50IGluCj4+Pj4+IGF0aDEwa193bWlfY21kX3NlbmQoKSBhZnRlciBhIGNv bW1hbmQgaXMgc2VudCBpbnN0ZWFkIG9mIGFsbCB0aGVzZQo+Pj4+PiBwcmludHMuIFRoaXMgd2F5 IHlvdSBjYW4gZ2V0IGEgZnVsbCBjYWxsIHRyYWNlIGlmIGl0IHRpbWVzIG91dC4KPj4+Pgo+Pj4+ Cj4+Pj4gV291bGQgdGhhdCBiZSBhIHBlcmZvcm1hbmNlIHByb2JsZW0gKGJ5IGVmZmVjdGl2ZWx5 IGFsbG93aW5nIG9ubHkgYQo+Pj4+IHNpbmdsZQo+Pj4+IGl0ZW0gdG8gYmUgc2VudCB0byB0aGUg ZmlybXdhcmUgYXQgYW55IGdpdmVuIHRpbWU/KQo+Pj4KPj4+Cj4+PiBUaGUgb25seSB0aGluZyB0 aGF0IG1heSBzdWZmZXIgaGVyZSBpcyB3bWkgbWdtdCB0eCBidXQgSSB3b3VsZG4ndAo+Pj4gY29u c2lkZXIgdGhhdCBwZXJmb3JtYW5jZSBjcml0aWNhbC4KPj4KPj4KPj4gSW4gdGhhdCBjYXNlLCB3 aHkgdXNlIGNyZWRpdHMgYXQgYWxsPwo+Cj4gRXhjZWxsZW50IHF1ZXN0aW9uLgo+Cj4gRmlybXdh cmUgcHJvdmlkZXMgMiBIVEMgdHggY3JlZGl0cyBmb3IgV01JLiBJIGd1ZXNzIHRoaXMgaXMgb24K PiBwdXJwb3NlLiBJIGltYWdpbmUgV01JIG1nbXQgdHggY2FuIGhhbmcgKGkuZS4gbm90IHJlcGxl bmlzaCB0eAo+IGNyZWRpdHMpLiBJbiB0aGF0IGNhc2UgdGhlIHJlbWFpbmluZyB0eCBjcmVkaXQg Y291bGQgYmUgdXNlZCB0byB0cnkKPiByZWNvdmVyaW5nLCBlLmcuIGRvd24vc3RvcCB2ZGV2IChJ IHRoaW5rIGl0IHNob3VsZCBkcm9wIGFsbCBwZW5kaW5nIHR4Cj4gYW5kIHRodXMgZnJlZSB0aGUg c3R1Y2sgSFRDIHR4IGNyZWRpdCkuCj4KPiBJdCBtaWdodCBiZSBhIGdvb2QgaWRlYSB0byBrZWVw IHRoaXMgcmVjb3Zlcnkgc3RyYXRlZ3kgaW4gbWluZCB3aGVuCj4gaW1wbGVtZW50aW5nIHRoZSAi d2FpdCBmb3IgdHggcmVwbGVuaXNobWVudCIgd21pIHR4IGJsb2NraW5nLgoKU2VlbXMgdG8gbWUg d2Ugc2hvdWxkIGp1c3QgZml4IHRoZSBmaXJtd2FyZSB0byByZXNwb25kIHF1aWNrbHkuLmFuZCBp ZiB3ZSBrbm93IGV4YWN0bHkgd2hpY2gKY29tbWFuZCBpcyBub3QgYmVpbmcgYW5zd2VyZWQgKHF1 aWNrbHkpIGluIHRoZSBmaXJtd2FyZSwgaXQgbWlnaHQgYmUgZWFzaWVyIHRvIGRlYnVnCml0LgoK VGhhdCBzYWlkLCB3aGlsZSBkZWJ1Z2dpbmcgSSB0aGluayBJIGhhdmUgc2VlbiBjYXNlcyB3aGVy ZSBvbmUgbWVzc2FnZSB3b3VsZApiZSBjb25zdW1lZCBmb3IgYSBsb25nIHRpbWUsIGFuZCB0aGVu IHN1ZGRlbmx5IGl0IGZyZWVzIHVwIGFnYWluLgoKU28sIEknbSBub3Qgc3VyZSB3aGF0IHRvIGRv LiAgTWF5YmUgY291bGQgaW1wcm92ZSBmaXJtd2FyZSB0byByZXR1cm4gYW4gaWQKc28gd2Uga25v dyBleGFjdGx5IHdoaWNoIG9mIHRoZSBtZXNzYWdlcyB3YXMgcHJvY2Vzc2VkLCBhbmQgdGhlbiB0 aGUgZHJpdmVyCmNvdWxkIGRvIHRoZSB3b3JrIG9mIGZpZ3VyaW5nIG91dCB3aGF0IGNvbW1hbmRz IGFyZSBvdXRzdGFuZGluZyBieSBqdXN0IGtlZXBpbmcKYSBjb3B5IGluIFJBTS4uLi4KClRoYW5r cywKQmVuCgo+Cj4KPiBNaWNoYcWCCj4KCgotLSAKQmVuIEdyZWVhciA8Z3JlZWFyYkBjYW5kZWxh dGVjaC5jb20+CkNhbmRlbGEgVGVjaG5vbG9naWVzIEluYyAgaHR0cDovL3d3dy5jYW5kZWxhdGVj aC5jb20KCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwph dGgxMGsgbWFpbGluZyBsaXN0CmF0aDEwa0BsaXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0 cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vYXRoMTBrCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.candelatech.com ([208.74.158.172]:52885 "EHLO ns3.lanforge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754089AbaCZOkp (ORCPT ); Wed, 26 Mar 2014 10:40:45 -0400 Message-ID: <5332E6C6.7040700@candelatech.com> (sfid-20140326_154050_928838_74B09378) Date: Wed, 26 Mar 2014 07:40:06 -0700 From: Ben Greear MIME-Version: 1.0 To: Michal Kazior CC: Kalle Valo , linux-wireless , "ath10k@lists.infradead.org" Subject: Re: [PATCH 1/3] ath10k: Add debugging for tx-credits usage. References: <1395428150-31996-1-git-send-email-greearb@candelatech.com> <871txr7svw.fsf@kamboji.qca.qualcomm.com> <53306239.2020006@candelatech.com> <53319D65.4090907@candelatech.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: On 03/26/2014 12:18 AM, Michal Kazior wrote: > On 25 March 2014 16:14, Ben Greear wrote: >> On 03/25/2014 12:27 AM, Michal Kazior wrote: >>> >>> On 24 March 2014 17:50, Ben Greear wrote: >>>> >>>> On 03/24/2014 04:21 AM, Michal Kazior wrote: >>>>> >>>>> On 24 March 2014 12:12, Kalle Valo wrote: >>>> >>>> >>>>>>> - status = ath10k_htc_send(htc, ATH10K_HTC_EP_0, skb); >>>>>>> + status = ath10k_htc_send(htc, ATH10K_HTC_EP_0, skb, __LINE__); >>>>>> >>>>>> >>>>>> Using line numbers in debug messages is very cumbersome. Some people >>>>>> cherry pick patches, have their own changes and whatnot which will make >>>>>> it more difficult to read the debug logs. Isn't there any better way to >>>>>> do this? >>>>> >>>>> >>>>> I would prefer to explicitly wait for tx credit replenishment in >>>>> ath10k_wmi_cmd_send() after a command is sent instead of all these >>>>> prints. This way you can get a full call trace if it times out. >>>> >>>> >>>> Would that be a performance problem (by effectively allowing only a >>>> single >>>> item to be sent to the firmware at any given time?) >>> >>> >>> The only thing that may suffer here is wmi mgmt tx but I wouldn't >>> consider that performance critical. >> >> >> In that case, why use credits at all? > > Excellent question. > > Firmware provides 2 HTC tx credits for WMI. I guess this is on > purpose. I imagine WMI mgmt tx can hang (i.e. not replenish tx > credits). In that case the remaining tx credit could be used to try > recovering, e.g. down/stop vdev (I think it should drop all pending tx > and thus free the stuck HTC tx credit). > > It might be a good idea to keep this recovery strategy in mind when > implementing the "wait for tx replenishment" wmi tx blocking. Seems to me we should just fix the firmware to respond quickly..and if we know exactly which command is not being answered (quickly) in the firmware, it might be easier to debug it. That said, while debugging I think I have seen cases where one message would be consumed for a long time, and then suddenly it frees up again. So, I'm not sure what to do. Maybe could improve firmware to return an id so we know exactly which of the messages was processed, and then the driver could do the work of figuring out what commands are outstanding by just keeping a copy in RAM.... Thanks, Ben > > > Michał > -- Ben Greear Candela Technologies Inc http://www.candelatech.com