Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1633168 > unrolled thread

[PATCH 0/3] dynamic_hex_dump cleanup

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-28 23:40 +0200
    [PATCH 2/3] virtio, rpmsg: switch to dynamic_hex_dump() Dan Williams <dan.j.williams@intel.com> - 2017-04-28 23:40 +0200
    [PATCH 3/3] acpi, nfit: kill ACPI_NFIT_DEBUG,  switch to dynamic_hex_dump() Dan Williams <dan.j.williams@intel.com> - 2017-04-28 23:40 +0200
      [PATCH v2] acpi, nfit: kill ACPI_NFIT_DEBUG Dan Williams <dan.j.williams@intel.com> - 2017-04-29 02:00 +0200
    Re: [PATCH 0/3] dynamic_hex_dump cleanup Joe Perches <joe@perches.com> - 2017-04-28 23:50 +0200
      Re: [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-29 00:00 +0200
        Re: [PATCH 0/3] dynamic_hex_dump cleanup Joe Perches <joe@perches.com> - 2017-04-29 00:20 +0200
          Re: [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-29 00:20 +0200
            Re: [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-29 00:30 +0200
              Re: [PATCH 0/3] dynamic_hex_dump cleanup Joe Perches <joe@perches.com> - 2017-04-29 00:40 +0200
                Re: [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-29 01:00 +0200
                  Re: [PATCH 0/3] dynamic_hex_dump cleanup Joe Perches <joe@perches.com> - 2017-04-29 01:10 +0200
                    Re: [PATCH 0/3] dynamic_hex_dump cleanup Dan Williams <dan.j.williams@intel.com> - 2017-04-29 01:20 +0200

#1633168 — [PATCH 0/3] dynamic_hex_dump cleanup

More than one driver has worked around the fact that
print_hex_dump_debug() requires CONFIG_DYNAMIC_DEBUG=y to build. 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'm fine to carry this whole set through nvdimm.git if no one sees any
issues.

---

Dan Williams (3):
      printk: provide generic dynamic_hex_dump fallback
      virtio, rpmsg: switch to dynamic_hex_dump()
      acpi, nfit: kill ACPI_NFIT_DEBUG, switch to dynamic_hex_dump()


 drivers/acpi/nfit/Kconfig        |   12 ------------
 drivers/acpi/nfit/core.c         |   21 ++++++++-------------
 drivers/rpmsg/virtio_rpmsg_bus.c |    6 ------
 include/linux/dynamic_debug.h    |    6 ++++++
 4 files changed, 14 insertions(+), 31 deletions(-)

[toc] | [next] | [standalone]

#1633169 — [PATCH 2/3] virtio, rpmsg: switch to dynamic_hex_dump()

Now that dynamic_hex_dump() has a fallback in the CONFIG_DYNAMIC_DEBUG=n
case we can kill the unnecessary ifdefs in the rpmsg driver.

Cc: Ohad Ben-Cohen <ohad@wizery.com>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/rpmsg/virtio_rpmsg_bus.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 5e66e081027e..00d92b399ddd 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -599,10 +599,8 @@ static int rpmsg_send_offchannel_raw(struct rpmsg_device *rpdev,
 
 	dev_dbg(dev, "TX From 0x%x, To 0x%x, Len %d, Flags %d, Reserved %d\n",
 		msg->src, msg->dst, msg->len, msg->flags, msg->reserved);
-#if defined(CONFIG_DYNAMIC_DEBUG)
 	dynamic_hex_dump("rpmsg_virtio TX: ", DUMP_PREFIX_NONE, 16, 1,
 			 msg, sizeof(*msg) + msg->len, true);
-#endif
 
 	sg_init_one(&sg, msg, sizeof(*msg) + len);
 
@@ -687,10 +685,8 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 
 	dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n",
 		msg->src, msg->dst, msg->len, msg->flags, msg->reserved);
-#if defined(CONFIG_DYNAMIC_DEBUG)
 	dynamic_hex_dump("rpmsg_virtio RX: ", DUMP_PREFIX_NONE, 16, 1,
 			 msg, sizeof(*msg) + msg->len, true);
-#endif
 
 	/*
 	 * We currently use fixed-sized buffers, so trivially sanitize
@@ -801,10 +797,8 @@ static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
 	struct device *dev = &vrp->vdev->dev;
 	int ret;
 
-#if defined(CONFIG_DYNAMIC_DEBUG)
 	dynamic_hex_dump("NS announcement: ", DUMP_PREFIX_NONE, 16, 1,
 			 data, len, true);
-#endif
 
 	if (len != sizeof(*msg)) {
 		dev_err(dev, "malformed ns msg (%d)\n", len);

[toc] | [prev] | [next] | [standalone]

#1633170 — [PATCH 3/3] acpi, nfit: kill ACPI_NFIT_DEBUG, switch to dynamic_hex_dump()

Inevitably when one actually needs to debug a DSM issue it's on a
distribution kernel that has CONFIG_ACPI_NFIT_DEBUG=n. The config symbol
is only there to avoid the compile error due to the missing fallback for
print_hex_dump_debug in the CONFIG_DYNAMIC_DEBUG=n case. Now that the
compile error is fixed the nfit driver can call dynamic_hex_dump()
directly, and enable debug of unmodified distro kernels.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/Kconfig |   12 ------------
 drivers/acpi/nfit/core.c  |   21 ++++++++-------------
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/nfit/Kconfig b/drivers/acpi/nfit/Kconfig
index dd0d53c52552..6d3351452ea2 100644
--- a/drivers/acpi/nfit/Kconfig
+++ b/drivers/acpi/nfit/Kconfig
@@ -12,15 +12,3 @@ config ACPI_NFIT
 
 	  To compile this driver as a module, choose M here:
 	  the module will be called nfit.
-
-config ACPI_NFIT_DEBUG
-	bool "NFIT DSM debug"
-	depends on ACPI_NFIT
-	depends on DYNAMIC_DEBUG
-	default n
-	help
-	  Enabling this option causes the nfit driver to dump the
-	  input and output buffers of _DSM operations on the ACPI0012
-	  device and its children.  This can be very verbose, so leave
-	  it disabled unless you are debugging a hardware / firmware
-	  issue.
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 261eea1d2906..ca391a2cbe63 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -268,14 +268,11 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		in_buf.buffer.length = call_pkg->nd_size_in;
 	}
 
-	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %d: func: %d input length: %d\n",
-				__func__, dimm_name, cmd, func,
-				in_buf.buffer.length);
-		print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
+	dev_dbg(dev, "%s:%s cmd: %d: func: %d input length: %d\n",
+			__func__, dimm_name, cmd, func, in_buf.buffer.length);
+	dynamic_hex_dump("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
 			in_buf.buffer.pointer,
 			min_t(u32, 256, in_buf.buffer.length), true);
-	}
 
 	out_obj = acpi_evaluate_dsm(handle, uuid, 1, func, &in_obj);
 	if (!out_obj) {
@@ -307,13 +304,11 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		goto out;
 	}
 
-	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %s output length: %d\n", __func__,
-				dimm_name, cmd_name, out_obj->buffer.length);
-		print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4,
-				4, out_obj->buffer.pointer, min_t(u32, 128,
-					out_obj->buffer.length), true);
-	}
+	dev_dbg(dev, "%s:%s cmd: %s output length: %d\n", __func__, dimm_name,
+			cmd_name, out_obj->buffer.length);
+	dynamic_hex_dump(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
+			out_obj->buffer.pointer,
+			min_t(u32, 128, out_obj->buffer.length), true);
 
 	for (i = 0, offset = 0; i < desc->out_num; i++) {
 		u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i, buf,

[toc] | [prev] | [next] | [standalone]

#1633207 — [PATCH v2] acpi, nfit: kill ACPI_NFIT_DEBUG

Inevitably when one actually needs to debug a DSM issue it's on a
distribution kernel that has CONFIG_ACPI_NFIT_DEBUG=n. The config symbol
was only there to avoid the compile error due to the missing fallback for
print_hex_dump_debug in the CONFIG_DYNAMIC_DEBUG=n case. That was fixed
with commit cdf17449af1d "hexdump: do not print debug dumps for
!CONFIG_DEBUG", so the config symbol can just be dropped.

Cc: Joe Perches <joe@perches.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes in v2:
* Thanks to Joe for pointing out the misunderstanding about
  print_hex_dump_debug() compilation being broken in the
  CONFIG_DYNAMIC_DEBUG=n case. Since commit cdf17449af1d we can rely on
  print_hex_dump_debug() to do the right thing by default.

 drivers/acpi/nfit/Kconfig |   12 ------------
 drivers/acpi/nfit/core.c  |   21 ++++++++-------------
 2 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/nfit/Kconfig b/drivers/acpi/nfit/Kconfig
index dd0d53c52552..6d3351452ea2 100644
--- a/drivers/acpi/nfit/Kconfig
+++ b/drivers/acpi/nfit/Kconfig
@@ -12,15 +12,3 @@ config ACPI_NFIT
 
 	  To compile this driver as a module, choose M here:
 	  the module will be called nfit.
-
-config ACPI_NFIT_DEBUG
-	bool "NFIT DSM debug"
-	depends on ACPI_NFIT
-	depends on DYNAMIC_DEBUG
-	default n
-	help
-	  Enabling this option causes the nfit driver to dump the
-	  input and output buffers of _DSM operations on the ACPI0012
-	  device and its children.  This can be very verbose, so leave
-	  it disabled unless you are debugging a hardware / firmware
-	  issue.
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 261eea1d2906..ced95a79c1a1 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -268,14 +268,11 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		in_buf.buffer.length = call_pkg->nd_size_in;
 	}
 
-	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %d: func: %d input length: %d\n",
-				__func__, dimm_name, cmd, func,
-				in_buf.buffer.length);
-		print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
+	dev_dbg(dev, "%s:%s cmd: %d: func: %d input length: %d\n",
+			__func__, dimm_name, cmd, func, in_buf.buffer.length);
+	print_hex_dump_debug("nvdimm in  ", DUMP_PREFIX_OFFSET, 4, 4,
 			in_buf.buffer.pointer,
 			min_t(u32, 256, in_buf.buffer.length), true);
-	}
 
 	out_obj = acpi_evaluate_dsm(handle, uuid, 1, func, &in_obj);
 	if (!out_obj) {
@@ -307,13 +304,11 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
 		goto out;
 	}
 
-	if (IS_ENABLED(CONFIG_ACPI_NFIT_DEBUG)) {
-		dev_dbg(dev, "%s:%s cmd: %s output length: %d\n", __func__,
-				dimm_name, cmd_name, out_obj->buffer.length);
-		print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4,
-				4, out_obj->buffer.pointer, min_t(u32, 128,
-					out_obj->buffer.length), true);
-	}
+	dev_dbg(dev, "%s:%s cmd: %s output length: %d\n", __func__, dimm_name,
+			cmd_name, out_obj->buffer.length);
+	print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4,
+			out_obj->buffer.pointer,
+			min_t(u32, 128, out_obj->buffer.length), true);
 
 	for (i = 0, offset = 0; i < desc->out_num; i++) {
 		u32 out_size = nd_cmd_out_size(nvdimm, cmd, desc, i, buf,

[toc] | [prev] | [next] | [standalone]

#1633173

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.

[toc] | [prev] | [next] | [standalone]

#1633176

On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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.

[toc] | [prev] | [next] | [standalone]

#1633181

On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote:
> On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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.

[toc] | [prev] | [next] | [standalone]

#1633183

On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote:
>> On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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.

[toc] | [prev] | [next] | [standalone]

#1633187

On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches <joe@perches.com> wrote:
>> On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote:
>>> On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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?

[toc] | [prev] | [next] | [standalone]

#1633193

On Fri, 2017-04-28 at 15:19 -0700, Dan Williams wrote:
> On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> > On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches <joe@perches.com> wrote:
> > > On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote:
> > > > On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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.

[toc] | [prev] | [next] | [standalone]

#1633200

On Fri, Apr 28, 2017 at 3:31 PM, Joe Perches <joe@perches.com> wrote:
> On Fri, 2017-04-28 at 15:19 -0700, Dan Williams wrote:
>> On Fri, Apr 28, 2017 at 3:14 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> > On Fri, Apr 28, 2017 at 3:07 PM, Joe Perches <joe@perches.com> wrote:
>> > > On Fri, 2017-04-28 at 14:52 -0700, Dan Williams wrote:
>> > > > On Fri, Apr 28, 2017 at 2:49 PM, Joe Perches <joe@perches.com> 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().

[toc] | [prev] | [next] | [standalone]

#1633201

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.

[toc] | [prev] | [next] | [standalone]

#1633203

On Fri, Apr 28, 2017 at 4:04 PM, Joe Perches <joe@perches.com> 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!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web