:כתב Kalle Valo, 2016-04-15 15:28 בתאריך
"Haim, Maya" <merez@xxxxxxxxxxxxxxxx> writes:
On 4/7/2016 6:41 PM, Kalle Valo wrote:
"Haim, Maya" <merez@xxxxxxxxxxxxxxxx> writes:
On 4/6/2016 10:19 AM, Joe Perches wrote:
On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
Add __func__ to all wil log macros for easier debugging.
I think this is unnecessary and merely bloats code size.
For all the _dbg calls, dynamic debug can add function names if
desired.
If really desired, I suggest changing the logging functions to use
"%ps and __builtin_return_address(0)
I implemented it with __builtin_return_address(0) at first but found
its format less readable (e.g. wil_start_xmit+0x58/0x7e8).
Will that work with inline functions and with functions which the
compiler has optimised out?
That's a good point. I did a quick check and it doesn't work for
inline or static functions - for such functions, the name of the
calling function is printed.
Thanks for checking.
We can either (1) use my initial implementation (2) add __func__ only
to the wil_dbg_... macros or (3) revert this patch completely. I find
the addition of the function names very useful and since most of the
code doesn't include it, it makes the analysis of issues less
efficient. Kalle - what is your say on that?
I don't have any preference, it's up to you what you like most.
One more possibility: in ath10k we have a kconfig option
CONFIG_ATH10K_DEBUG to make it possible to disable all overhead from
debug functionality, that would at least solve Joe's concern of extra
memory usage.
I looked a bit more on the different options.
For the dbg functions, the function name can be added as part of the
dynamic debug options (as mentioned by Joe). Therefore I will remove
the addition of __func__ from those macros.
The dynamic debug also guarantees zero overhead so I don't see the need
for an additional debug config flag (such as CONFIG_ATH10K_DEBUG).
As for wil_err and wil_info, I believe we can add the __func__ to them,
as wil_info is not commonly used in our code.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
