Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, Apr 28, 2017 at 4:04 PM, Joe Perches wrote: > On Fri, 2017-04-28 at 15:59 -0700, Dan Williams wrote: >> ...but again, hex_dump_dbg() implies to me dev_dbg() behavior, i.e. >> that the hexdump happens in the KERN_DEBUG case > > I think you are confusing KERN_DEBUG, a logging level, with a > preprocessor #define of DEBUG. Indeed I am. > >> which I don't want. I >> want the call to be compiled out completely when it can't be >> enabled/disabled dynamically. > > dev_dbg is compiled away completely when DEBUG is > not defined and when CONFIG_DYNAMIC_DEBUG is not > defined. > > This hex_dump facility should behave the same way. > >> I think we do need a hex_dump_dbg(), but that does not eliminate the >> utility of / need for dynamic_hex_dump(). > > dynamic_hex_dump should only exist when CONFIG_DYNAMIC_DEBUG > is enabled and should otherwise be invisible to driver code. > So, I overlooked that Linus fixed this compile problem a while back with: cdf17449af1d hexdump: do not print debug dumps for !CONFIG_DEBUG Which predated the introduction of ACPI_NFIT_DEBUG. So I can just call print_hex_dump_debug() and call it a day. Sorry for the noise, Joe!
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, 2017-04-28 at 15:59 -0700, Dan Williams wrote: > ...but again, hex_dump_dbg() implies to me dev_dbg() behavior, i.e. > that the hexdump happens in the KERN_DEBUG case I think you are confusing KERN_DEBUG, a logging level, with a preprocessor #define of DEBUG. > which I don't want. I > want the call to be compiled out completely when it can't be > enabled/disabled dynamically. dev_dbg is compiled away completely when DEBUG is not defined and when CONFIG_DYNAMIC_DEBUG is not defined. This hex_dump facility should behave the same way. > I think we do need a hex_dump_dbg(), but that does not eliminate the > utility of / need for dynamic_hex_dump(). dynamic_hex_dump should only exist when CONFIG_DYNAMIC_DEBUG is enabled and should otherwise be invisible to driver code.
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, Apr 28, 2017 at 3:31 PM, Joe Perches wrote: > On Fri, 2017-04-28 at 15:19 -0700, Dan Williams wrote: >> On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams >> wrote: >> > On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches wrote: >> > > On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote: >> > > > On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: >> > > > > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: >> > > > > > More than one driver has worked around the fact that >> > > > > > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. >> > > > > >> > > > > No it doesn't. builds work fine. Output is restricted. >> > > > > >> > > > > > Provide a dynamic_hex_dump() so that drivers that want the extra >> > > > > > debugging to be >> > > > > > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() >> > > > > > directly. >> > > > > >> > > > > I think the concept is unnecessary >> > > > > . >> > > > > Just use print_hex_dump with KERN_DEBUG. >> > > > >> > > > No, we don't want any possibility of output in the >> > > > CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense >> > > > in the CONFIG_DYNAMIC_DEBUG case. >> > > >> > > No, that doesn't work the same. >> > > >> > > Look at your conversion of drivers/acpi/nfit/core.c >> > > >> > > dev_dbg outputs always when DEBUG is defined and >> > > optionally outputs when CONFIG_DYNAMIC_DEBUG is enabled. >> > >> > Right, that's what I want dev_dbg() to output at KERN_DEBUG level >> > always and the hexdump only in the dynamic case. >> >> Joe, I'm trying to understand your objection. What you are proposing >> is that the debug output is present at the KERN_DEBUG level in the >> CONFIG_DYNAMIC_DEBUG=n case and that's not what either use case wants. >> What breaks by letting users call dynamic_hex_dump() directly? > > Call it hex_dump_dbg and I have no objections. ...but again, hex_dump_dbg() implies to me dev_dbg() behavior, i.e. that the hexdump happens in the KERN_DEBUG case which I don't want. I want the call to be compiled out completely when it can't be enabled/disabled dynamically. > The "dynamicism" is a side effect of not having DEBUG > defined. dev_dbg will emit if DEBUG is declared regardless > of dynamic_debug. > > This new function should have the same behavior. I think we do need a hex_dump_dbg(), but that does not eliminate the utility of / need for dynamic_hex_dump().
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, 2017-04-28 at 15:19 -0700, Dan Williams wrote: > On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams > wrote: > > On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches wrote: > > > On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote: > > > > On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: > > > > > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: > > > > > > More than one driver has worked around the fact that > > > > > > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. > > > > > > > > > > No it doesn't. builds work fine. Output is restricted. > > > > > > > > > > > Provide a dynamic_hex_dump() so that drivers that want the extra > > > > > > debugging to be > > > > > > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() > > > > > > directly. > > > > > > > > > > I think the concept is unnecessary > > > > > . > > > > > Just use print_hex_dump with KERN_DEBUG. > > > > > > > > No, we don't want any possibility of output in the > > > > CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense > > > > in the CONFIG_DYNAMIC_DEBUG case. > > > > > > No, that doesn't work the same. > > > > > > Look at your conversion of drivers/acpi/nfit/core.c > > > > > > dev_dbg outputs always when DEBUG is defined and > > > optionally outputs when CONFIG_DYNAMIC_DEBUG is enabled. > > > > Right, that's what I want dev_dbg() to output at KERN_DEBUG level > > always and the hexdump only in the dynamic case. > > Joe, I'm trying to understand your objection. What you are proposing > is that the debug output is present at the KERN_DEBUG level in the > CONFIG_DYNAMIC_DEBUG=n case and that's not what either use case wants. > What breaks by letting users call dynamic_hex_dump() directly? Call it hex_dump_dbg and I have no objections. The "dynamicism" is a side effect of not having DEBUG defined. dev_dbg will emit if DEBUG is declared regardless of dynamic_debug. This new function should have the same behavior.
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams wrote: > On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches wrote: >> On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote: >>> On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: >>> > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: >>> > > More than one driver has worked around the fact that >>> > > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. >>> > >>> > No it doesn't. builds work fine. Output is restricted. >>> > >>> > > Provide a dynamic_hex_dump() so that drivers that want the extra >>> > > debugging to be >>> > > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() >>> > > directly. >>> > >>> > I think the concept is unnecessary >>> > . >>> > Just use print_hex_dump with KERN_DEBUG. >>> >>> No, we don't want any possibility of output in the >>> CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense >>> in the CONFIG_DYNAMIC_DEBUG case. >> >> No, that doesn't work the same. >> >> Look at your conversion of drivers/acpi/nfit/core.c >> >> dev_dbg outputs always when DEBUG is defined and >> optionally outputs when CONFIG_DYNAMIC_DEBUG is enabled. > > Right, that's what I want dev_dbg() to output at KERN_DEBUG level > always and the hexdump only in the dynamic case. Joe, I'm trying to understand your objection. What you are proposing is that the debug output is present at the KERN_DEBUG level in the CONFIG_DYNAMIC_DEBUG=n case and that's not what either use case wants. What breaks by letting users call dynamic_hex_dump() directly?
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches wrote: > On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote: >> On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: >> > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: >> > > More than one driver has worked around the fact that >> > > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. >> > >> > No it doesn't. builds work fine. Output is restricted. >> > >> > > Provide a dynamic_hex_dump() so that drivers that want the extra >> > > debugging to be >> > > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() >> > > directly. >> > >> > I think the concept is unnecessary >> > . >> > Just use print_hex_dump with KERN_DEBUG. >> >> No, we don't want any possibility of output in the >> CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense >> in the CONFIG_DYNAMIC_DEBUG case. > > No, that doesn't work the same. > > Look at your conversion of drivers/acpi/nfit/core.c > > dev_dbg outputs always when DEBUG is defined and > optionally outputs when CONFIG_DYNAMIC_DEBUG is enabled. Right, that's what I want dev_dbg() to output at KERN_DEBUG level always and the hexdump only in the dynamic case.
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote: > On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: > > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: > > > More than one driver has worked around the fact that > > > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. > > > > No it doesn't. builds work fine. Output is restricted. > > > > > Provide a dynamic_hex_dump() so that drivers that want the extra > > > debugging to be > > > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() > > > directly. > > > > I think the concept is unnecessary > > . > > Just use print_hex_dump with KERN_DEBUG. > > No, we don't want any possibility of output in the > CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense > in the CONFIG_DYNAMIC_DEBUG case. No, that doesn't work the same. Look at your conversion of drivers/acpi/nfit/core.c dev_dbg outputs always when DEBUG is defined and optionally outputs when CONFIG_DYNAMIC_DEBUG is enabled.
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches wrote: > On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: >> More than one driver has worked around the fact that >> print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. > > No it doesn't. builds work fine. Output is restricted. > >> Provide a dynamic_hex_dump() so that drivers that want the extra debugging >> to be >> turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() >> directly. > > I think the concept is unnecessary > . > Just use print_hex_dump with KERN_DEBUG. No, we don't want any possibility of output in the CONFIG_DYNAMIC_DEBUG=n case. This is extra debug that only makes sense in the CONFIG_DYNAMIC_DEBUG case.
Re: [PATCH 0/3] dynamic_hex_dump cleanup
On Fri, 2017-04-28 at 14:28 -0700, Dan Williams wrote: > More than one driver has worked around the fact that > print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. No it doesn't. builds work fine. Output is restricted. > Provide a dynamic_hex_dump() so that drivers that want the extra debugging to > be > turned off in the CONFIG_DYNAMIC_DEBUG=n can use dynamic_hex_dump() > directly. I think the concept is unnecessary . Just use print_hex_dump with KERN_DEBUG.
