| Message ID | 20200513070843.71528-1-hare@suse.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | device-mapper: use dynamic debug instead of compile-time config option | expand |
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@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 ? > -#define DMDEBUG(fmt, ...) printk(KERN_DEBUG DM_FMT(fmt), ##__VA_ARGS__) > +#define DMDEBUG(fmt, ...) pr_debug(DM_FMT(fmt), ##__VA_ARGS__) > #define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), ##__VA_ARGS__) > -#else > -#define DMDEBUG(fmt, ...) no_printk(fmt, ##__VA_ARGS__) > -#define DMDEBUG_LIMIT(fmt, ...) no_printk(fmt, ##__VA_ARGS__) > -#endif > > #define DMEMIT(x...) sz += ((sz >= maxlen) ? \ > 0 : scnprintf(result + sz, maxlen - sz, x)) > Otherwise, looks good to me. Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
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@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. Cheers, Hannes
On Wed, May 13 2020 at 7:10am -0400, Hannes Reinecke <hare@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@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? Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 5/13/20 6:25 PM, Mike Snitzer wrote: > On Wed, May 13 2020 at 7:10am -0400, > Hannes Reinecke <hare@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@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. Cheers, Hannes
On Wed, May 13 2020 at 1:01pm -0400, Hannes Reinecke <hare@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@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@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. Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 5/13/20 7:10 PM, Mike Snitzer wrote: > On Wed, May 13 2020 at 1:01pm -0400, > Hannes Reinecke <hare@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@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@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
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 -#define DMDEBUG(fmt, ...) printk(KERN_DEBUG DM_FMT(fmt), ##__VA_ARGS__) +#define DMDEBUG(fmt, ...) pr_debug(DM_FMT(fmt), ##__VA_ARGS__) #define DMDEBUG_LIMIT(fmt, ...) pr_debug_ratelimited(DM_FMT(fmt), ##__VA_ARGS__) -#else -#define DMDEBUG(fmt, ...) no_printk(fmt, ##__VA_ARGS__) -#define DMDEBUG_LIMIT(fmt, ...) no_printk(fmt, ##__VA_ARGS__) -#endif #define DMEMIT(x...) sz += ((sz >= maxlen) ? \ 0 : scnprintf(result + sz, maxlen - sz, x))
Switch to use dynamic debug to avoid having recompile the kernel just to enable debugging messages. Signed-off-by: Hannes Reinecke <hare@suse.de> --- include/linux/device-mapper.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)