Re: [PATCH 1/7] wil6210: add function name to wil log macros

[merez@codeaurora.org]

:כתב Kalle Valo, 2016-04-15 15:28 בתאריך
> "Haim, Maya" <merez@qti.qualcomm.com> writes:
> 
>> On 4/7/2016 6:41 PM, Kalle Valo wrote:
>>> "Haim, Maya" <merez@qti.qualcomm.com> 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.