linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felix Fietkau <nbd@openwrt.org>
To: Ben Greear <greearb@candelatech.com>
Cc: Sujith <m.sujith@gmail.com>,
	ath9k-devel@venema.h4ckr.net, linux-wireless@vger.kernel.org,
	linville@tuxdriver.com
Subject: Re: [ath9k-devel] [PATCH 3/7] ath9k: Merge wiphy and misc debugfs files
Date: Tue, 14 Feb 2012 23:29:28 +0100	[thread overview]
Message-ID: <4F3AE048.5030503@openwrt.org> (raw)
In-Reply-To: <4F3AAB3F.7030308@candelatech.com>

On 2012-02-14 7:43 PM, Ben Greear wrote:
> On 02/14/2012 10:29 AM, Sujith wrote:
>> Ben Greear wrote:
>>> Actually, I think it might be useful to have a second level of debugging.
>>> I hope to soon have time&  resources to add some logic to dump lots of register
>>> info and such in human-readable format, (like, when DMA times out).  That is going to be a lot
>>> of strings added to the driver, so the compile size will definitely
>>> increase.  If keeping the size small is important, then this sort of verbose thing
>>> could be hidden behind a second level of debugging...
>>
>> That could be implemented similar to what usbmon does. A debugfs file that could
>> be read and redirected to a file. And there would be no overhead to the
>> driver, I think. We could call it the 'event log'. :)
> 
> I was thinking about adding a method that grabbed as many registers
> as I have info for and dumping them with printk when DMA errors
> hit.  This would make kernel splats more useful.
> 
> And also have a debugfs file called 'registers' or similar that one
> could cat out and get similar info.  And this can let folks look
> at steady-state or whatever.
> 
> But, the logic to turn the register bit values into strings would
> be in the driver (and thus add some code size bloat).
> 
> My hope is that this would allow a better chance of understanding
> the stop-DMA errors that some people get reliably (but which I can never reliably
> reproduce).
> 
> I'm not sure how that plays into your 'event log' idea, but maybe
> one will help the other.
I think the 'let's dump all kinds of random crap when the issue occurs
until we find somebody that can parse it' approach won't work here, and
I really think it's not a good idea in general.

In the past the stop-DMA crap has been a symptom with a wild variety of
different causes, most of which were actually *software* race
conditions, e.g. dma tx or rx enable during reset, locking issues, etc.

In addition to those software causes, there was one actual hardware
condition that also triggered this error, and even this wouldn't have
showed up in a normal register dump, because it required setting up the
MAC observation bus in a particular way.

That hardware trigger for this issue was analyzed not by dumping random
data, but by actually talking to hardware designers that could look
through the code and guide the debugging process.

Let's not carpet-bomb the driver with lots of debug crap that probably
won't ever lead anybody to any good solution for the remaining issues,
let's fix stuff the old-fashioned way: by reading the code,
understanding what's going on, analyzing problems in a systematic way,
rather than clouding the whole process with assumptions based on old
bugs that have since been fixed.

- Felix

  reply	other threads:[~2012-02-14 22:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-13  6:20 [PATCH 3/7] ath9k: Merge wiphy and misc debugfs files Sujith Manoharan
2012-02-13 17:25 ` Ben Greear
2012-02-14  1:48   ` Sujith Manoharan
2012-02-14  1:56     ` Ben Greear
2012-02-14  2:52       ` Sujith Manoharan
2012-02-14  7:38         ` [ath9k-devel] " Peter Stuge
2012-02-14  8:30           ` Sujith Manoharan
2012-02-14 17:32             ` Ben Greear
2012-02-14 18:29               ` Sujith
2012-02-14 18:43                 ` Ben Greear
2012-02-14 22:29                   ` Felix Fietkau [this message]
2012-02-14 22:41                     ` Ben Greear
2012-02-15  4:47                   ` Sujith Manoharan
2012-02-15  8:37                     ` Sujith Manoharan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F3AE048.5030503@openwrt.org \
    --to=nbd@openwrt.org \
    --cc=ath9k-devel@venema.h4ckr.net \
    --cc=greearb@candelatech.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=m.sujith@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).