Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled
On Thu, 2018-12-06 at 15:41 +0100, Michel Dänzer wrote: > On 2018-12-06 1:23 p.m., Joe Perches wrote: > > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote: > > > In contrast to the 2b case, the pr_debug output isn't visible by default > > > with 1b, so the latter doesn't fit "always produce output" either. > > > > I think you are mistaken here. > > Still puzzled as to what you're hoping to achieve with that kind of > language. None of the confusion about this patch has been on my part. :) Doubtful. > > Adding #define DEBUG as Chris did enables pr_debug output > > and is your 1b. > > > > Perhaps your default console logging level is set to a > > non-default value. > > I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed > by this patch is that messages from drm_debug_printer are visible by > default (case 2b), whereas they shouldn't be (case 2a, like 1b). The default for config_dynamic_debug when DEBUG is defined is to output the message equivalent to when DEBUG is defined and config_dynamic_debug is not. That's a good thing and has been the general default since 2011. You want to override that, I suppose that's OK but it's definitely not a _fix_ as you have written. It's just a change in behavior. You added a "Fixes:" line. I believe that wrong. A long time ago, at my suggestion: commit b558c96ffa53f4b3dd52b774e4fb7a52982ab52b Author: Jim Cromie Date: Mon Dec 19 17:11:18 2011 -0500 dynamic_debug: make dynamic-debug supersede DEBUG ccflag If CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that pr_debug()s are controllable, instead of always-on. When DEBUG is also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by default. cheers, Joe
Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled
On 2018-12-06 5:10 p.m., Daniel Thompson wrote: > On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote: >> On 2018-12-06 1:23 p.m., Joe Perches wrote: >>> On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote: In contrast to the 2b case, the pr_debug output isn't visible by default with 1b, so the latter doesn't fit "always produce output" either. >>> >>> I think you are mistaken here. >> >> Still puzzled as to what you're hoping to achieve with that kind of >> language. None of the confusion about this patch has been on my part. :) >> >> >>> Adding #define DEBUG as Chris did enables pr_debug output >>> and is your 1b. >>> >>> Perhaps your default console logging level is set to a >>> non-default value. >> >> I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed >> by this patch is that messages from drm_debug_printer are visible by >> default (case 2b), whereas they shouldn't be (case 2a, like 1b). > > When enabled (either dynamically or statically) pr_debug() will emit > output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG > is defined or not. > > Thus unless you change additional settings (either dynamically or > statically) then debug messages should not be shown on the console > because the default settings filter KERN_DEBUG messages. However they > are available via dmesg and system loggers (syslogd, journal, etc). > > The patch proposed will change the behaviour of the debug messages > w.r.t. system loggers based on whether the user has enabled > CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise. Ah, that makes sense now, thanks. I'm withdrawing this patch. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled
On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote: > On 2018-12-06 1:23 p.m., Joe Perches wrote: > > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote: > >> In contrast to the 2b case, the pr_debug output isn't visible by default > >> with 1b, so the latter doesn't fit "always produce output" either. > > > > I think you are mistaken here. > > Still puzzled as to what you're hoping to achieve with that kind of > language. None of the confusion about this patch has been on my part. :) > > > > Adding #define DEBUG as Chris did enables pr_debug output > > and is your 1b. > > > > Perhaps your default console logging level is set to a > > non-default value. > > I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed > by this patch is that messages from drm_debug_printer are visible by > default (case 2b), whereas they shouldn't be (case 2a, like 1b). When enabled (either dynamically or statically) pr_debug() will emit output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG is defined or not. Thus unless you change additional settings (either dynamically or statically) then debug messages should not be shown on the console because the default settings filter KERN_DEBUG messages. However they are available via dmesg and system loggers (syslogd, journal, etc). The patch proposed will change the behaviour of the debug messages w.r.t. system loggers based on whether the user has enabled CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise. Daniel.
Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled
On Thu, Dec 06, 2018 at 03:41:16PM +0100, Michel Dänzer wrote: > On 2018-12-06 1:23 p.m., Joe Perches wrote: > > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote: > >> In contrast to the 2b case, the pr_debug output isn't visible by default > >> with 1b, so the latter doesn't fit "always produce output" either. > > > > I think you are mistaken here. > > Still puzzled as to what you're hoping to achieve with that kind of > language. None of the confusion about this patch has been on my part. :) > > > > Adding #define DEBUG as Chris did enables pr_debug output > > and is your 1b. > > > > Perhaps your default console logging level is set to a > > non-default value. > > I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed > by this patch is that messages from drm_debug_printer are visible by > default (case 2b), whereas they shouldn't be (case 2a, like 1b). When enabled (either dynamically or statically) pr_debug() will emit output at KERN_DEBUG level regardless of whether CONFIG_DYNAMIC_DEBUG is defined or not. Thus unless you change additional settings (either dynamically or statically) then debug messages should not be shown on the console because the default settings filter KERN_DEBUG messages. However they are available via dmesg and system loggers (syslogd, journal, etc). The patch proposed will change the behaviour of the debug messages w.r.t. system loggers based on whether the user has enabled CONFIG_DYNAMIC_DEBUG or not, violating the principle of least surprise. Daniel.
Re: [PATCH 1/2] drm: Only #define DEBUG if CONFIG_DYNAMIC_DEBUG is disabled
On 2018-12-06 1:23 p.m., Joe Perches wrote: > On Thu, 2018-12-06 at 12:52 +0100, Michel Dänzer wrote: >> In contrast to the 2b case, the pr_debug output isn't visible by default >> with 1b, so the latter doesn't fit "always produce output" either. > > I think you are mistaken here. Still puzzled as to what you're hoping to achieve with that kind of language. None of the confusion about this patch has been on my part. :) > Adding #define DEBUG as Chris did enables pr_debug output > and is your 1b. > > Perhaps your default console logging level is set to a > non-default value. I have CONFIG_DYNAMIC_DEBUG enabled in my kernels. The problem addressed by this patch is that messages from drm_debug_printer are visible by default (case 2b), whereas they shouldn't be (case 2a, like 1b). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
