All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/1] devres: Enable trace events
@ 2021-04-13 11:38 Andy Shevchenko
  2021-04-13 12:00 ` Heikki Krogerus
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2021-04-13 11:38 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Bartosz Golaszewski,
	Andy Shevchenko, linux-kernel
  Cc: Rafael J. Wysocki

In some cases the printf() mechanism is too heavy and can't be used.
For example, when debugging a race condition involving devres API.
When CONFIG_DEBUG_DEVRES is enabled I can't reproduce an issue, and
otherwise it's quite visible with a useful information being collected.

Enable trace events for devres part of the driver core.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: fixed compilation error (lkp), elaborate commit message (Greg)
 drivers/base/Makefile |  3 +++
 drivers/base/devres.c | 23 +++++++++++-------
 drivers/base/trace.c  | 10 ++++++++
 drivers/base/trace.h  | 56 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 9 deletions(-)
 create mode 100644 drivers/base/trace.c
 create mode 100644 drivers/base/trace.h

diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 8b93a7f291ec..ef8e44a7d288 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -30,3 +30,6 @@ obj-y			+= test/
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
 
+# define_trace.h needs to know how to find our header
+CFLAGS_trace.o		:= -I$(src)
+obj-$(CONFIG_TRACING)	+= trace.o
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index db1f3137fc81..a0850bd1eab7 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -14,14 +14,13 @@
 #include <asm/sections.h>
 
 #include "base.h"
+#include "trace.h"
 
 struct devres_node {
 	struct list_head		entry;
 	dr_release_t			release;
-#ifdef CONFIG_DEBUG_DEVRES
 	const char			*name;
 	size_t				size;
-#endif
 };
 
 struct devres {
@@ -43,10 +42,6 @@ struct devres_group {
 	/* -- 8 pointers */
 };
 
-#ifdef CONFIG_DEBUG_DEVRES
-static int log_devres = 0;
-module_param_named(log, log_devres, int, S_IRUGO | S_IWUSR);
-
 static void set_node_dbginfo(struct devres_node *node, const char *name,
 			     size_t size)
 {
@@ -54,7 +49,11 @@ static void set_node_dbginfo(struct devres_node *node, const char *name,
 	node->size = size;
 }
 
-static void devres_log(struct device *dev, struct devres_node *node,
+#ifdef CONFIG_DEBUG_DEVRES
+static int log_devres = 0;
+module_param_named(log, log_devres, int, S_IRUGO | S_IWUSR);
+
+static void devres_dbg(struct device *dev, struct devres_node *node,
 		       const char *op)
 {
 	if (unlikely(log_devres))
@@ -62,10 +61,16 @@ static void devres_log(struct device *dev, struct devres_node *node,
 			op, node, node->name, node->size);
 }
 #else /* CONFIG_DEBUG_DEVRES */
-#define set_node_dbginfo(node, n, s)	do {} while (0)
-#define devres_log(dev, node, op)	do {} while (0)
+#define devres_dbg(dev, node, op)	do {} while (0)
 #endif /* CONFIG_DEBUG_DEVRES */
 
+static void devres_log(struct device *dev, struct devres_node *node,
+		       const char *op)
+{
+	trace_devres_log(dev, op, node, node->name, node->size);
+	devres_dbg(dev, node, op);
+}
+
 /*
  * Release functions for devres group.  These callbacks are used only
  * for identification.
diff --git a/drivers/base/trace.c b/drivers/base/trace.c
new file mode 100644
index 000000000000..b24b0a309c4a
--- /dev/null
+++ b/drivers/base/trace.c
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Device core Trace Support
+ * Copyright (C) 2021, Intel Corporation
+ *
+ * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ */
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
diff --git a/drivers/base/trace.h b/drivers/base/trace.h
new file mode 100644
index 000000000000..3192e18f877e
--- /dev/null
+++ b/drivers/base/trace.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Device core Trace Support
+ * Copyright (C) 2021, Intel Corporation
+ *
+ * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dev
+
+#if !defined(__DEV_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define __DEV_TRACE_H
+
+#include <linux/device.h>
+#include <linux/tracepoint.h>
+#include <linux/types.h>
+
+DECLARE_EVENT_CLASS(devres,
+	TP_PROTO(struct device *dev, const char *op, void *node, const char *name, size_t size),
+	TP_ARGS(dev, op, node, name, size),
+	TP_STRUCT__entry(
+		__string(devname, dev_name(dev))
+		__field(struct device *, dev)
+		__field(const char *, op)
+		__field(void *, node)
+		__field(const char *, name)
+		__field(size_t, size)
+	),
+	TP_fast_assign(
+		__assign_str(devname, dev_name(dev));
+		__entry->op = op;
+		__entry->node = node;
+		__entry->name = name;
+		__entry->size = size;
+	),
+	TP_printk("%s %3s %p %s (%zu bytes)", __get_str(devname),
+		  __entry->op, __entry->node, __entry->name, __entry->size)
+);
+
+DEFINE_EVENT(devres, devres_log,
+	TP_PROTO(struct device *dev, const char *op, void *node, const char *name, size_t size),
+	TP_ARGS(dev, op, node, name, size)
+);
+
+#endif /* __DEV_TRACE_H */
+
+/* this part has to be here */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+
+#include <trace/define_trace.h>
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] devres: Enable trace events
  2021-04-13 11:38 [PATCH v2 1/1] devres: Enable trace events Andy Shevchenko
@ 2021-04-13 12:00 ` Heikki Krogerus
  2021-04-13 12:25   ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Heikki Krogerus @ 2021-04-13 12:00 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, Bartosz Golaszewski, linux-kernel, Rafael J. Wysocki

On Tue, Apr 13, 2021 at 02:38:01PM +0300, Andy Shevchenko wrote:
> In some cases the printf() mechanism is too heavy and can't be used.
> For example, when debugging a race condition involving devres API.
> When CONFIG_DEBUG_DEVRES is enabled I can't reproduce an issue, and
> otherwise it's quite visible with a useful information being collected.
> 
> Enable trace events for devres part of the driver core.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: fixed compilation error (lkp), elaborate commit message (Greg)
>  drivers/base/Makefile |  3 +++
>  drivers/base/devres.c | 23 +++++++++++-------
>  drivers/base/trace.c  | 10 ++++++++
>  drivers/base/trace.h  | 56 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/base/trace.c
>  create mode 100644 drivers/base/trace.h
> 
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 8b93a7f291ec..ef8e44a7d288 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -30,3 +30,6 @@ obj-y			+= test/
>  
>  ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
>  
> +# define_trace.h needs to know how to find our header
> +CFLAGS_trace.o		:= -I$(src)
> +obj-$(CONFIG_TRACING)	+= trace.o
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index db1f3137fc81..a0850bd1eab7 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -14,14 +14,13 @@
>  #include <asm/sections.h>
>  
>  #include "base.h"
> +#include "trace.h"
>  
>  struct devres_node {
>  	struct list_head		entry;
>  	dr_release_t			release;
> -#ifdef CONFIG_DEBUG_DEVRES
>  	const char			*name;
>  	size_t				size;
> -#endif

Those ifdefs are still required.

>  };

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] devres: Enable trace events
  2021-04-13 12:00 ` Heikki Krogerus
@ 2021-04-13 12:25   ` Andy Shevchenko
  0 siblings, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2021-04-13 12:25 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Greg Kroah-Hartman, Bartosz Golaszewski, linux-kernel, Rafael J. Wysocki

On Tue, Apr 13, 2021 at 03:00:52PM +0300, Heikki Krogerus wrote:
> On Tue, Apr 13, 2021 at 02:38:01PM +0300, Andy Shevchenko wrote:
> > In some cases the printf() mechanism is too heavy and can't be used.
> > For example, when debugging a race condition involving devres API.
> > When CONFIG_DEBUG_DEVRES is enabled I can't reproduce an issue, and
> > otherwise it's quite visible with a useful information being collected.
> > 
> > Enable trace events for devres part of the driver core.

...

> >  struct devres_node {
> >  	struct list_head		entry;
> >  	dr_release_t			release;
> > -#ifdef CONFIG_DEBUG_DEVRES
> >  	const char			*name;
> >  	size_t				size;
> > -#endif
> 
> Those ifdefs are still required.

Oh, yes, otherwise we end up with not filled name and size (as per header
file). I'll rework in order to fill name and size always and get rid of
ifdeffery in the header file.

Thanks!

> >  };

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-13 12:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 11:38 [PATCH v2 1/1] devres: Enable trace events Andy Shevchenko
2021-04-13 12:00 ` Heikki Krogerus
2021-04-13 12:25   ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.