[dm-devel] device-mapper: use dynamic debug instead of compile-time config option

[Hannes Reinecke]

On 5/13/20 7:10 PM, Mike Snitzer wrote:
> On Wed, May 13 2020 at  1:01pm -0400,
> Hannes Reinecke <hare at suse.de> wrote:
> 
>> On 5/13/20 6:25 PM, Mike Snitzer wrote:
>>> On Wed, May 13 2020 at  7:10am -0400,
>>> Hannes Reinecke <hare at suse.de> wrote:
>>>
>>>> On 5/13/20 11:41 AM, Damien Le Moal wrote:
>>>>> On 2020/05/13 16:10, Hannes Reinecke wrote:
>>>>>> Switch to use dynamic debug to avoid having recompile the kernel
>>>>>> just to enable debugging messages.
>>>>>>
>>>>>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>>>>>> ---
>>>>>>   include/linux/device-mapper.h | 7 +------
>>>>>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
>>>>>> index e2d506dd805e..3d4365fd3001 100644
>>>>>> --- a/include/linux/device-mapper.h
>>>>>> +++ b/include/linux/device-mapper.h
>>>>>> @@ -556,13 +556,8 @@ void *dm_vcalloc(unsigned long nmemb, unsigned long elem_size);
>>>>>>   #define DMINFO(fmt, ...) pr_info(DM_FMT(fmt), ##__VA_ARGS__)
>>>>>>   #define DMINFO_LIMIT(fmt, ...) pr_info_ratelimited(DM_FMT(fmt), ##__VA_ARGS__)
>>>>>> -#ifdef CONFIG_DM_DEBUG
>>>>>
>>>>> Can we remove this from Kconfig as a config option ?
>>>>>
>>>> No, we can't, it's being used by dm-snap and dm-integrity.
>>>
>>> Yeah, they provide additional debugging if its set.
>>>
>>> But shouldn't we preserve old-style DMDEBUG if CONFIG_DM_DEBUG is set
>>> (compile time printing of debugging) but if not set, use dynamic
>>> debugging?
>>>
>>> Think I'd prefer that as the incremental improvement... thoughts?
>>>
>> Works for me; I just don't want to recompile the kernel anytime I
>> need to debug device-mapper stuff.
>>
>> Will be resending.
> 
> Does the use of pr_debug() cause missing newlines (with your patch)?
> 
> Only thought to ask this because I'm chasing another DM issue at the
> moment and happened to see __dm_suspend()'s use of pr_debug() includes
> an explicit extra \n.
> 
Hmm. Not that I've noticed; but yeah, typically one would have to add 
'\n' to any of the pr_XXX calls. But then it depends on the wrapper 
being used. I'll check.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare at suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer