* [PATCH v4 2/4] devres: Use list_for_each_safe_from() in remove_nodes()
2021-05-17 12:29 [PATCH v4 1/4] devres: Make locking straight forward in release_nodes() Andy Shevchenko
@ 2021-05-17 12:29 ` Andy Shevchenko
2021-05-17 12:29 ` [PATCH v4 3/4] devres: No need to call remove_nodes() when there none present Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2021-05-17 12:29 UTC (permalink / raw)
To: Heikki Krogerus, Bartosz Golaszewski, Andy Shevchenko,
Greg Kroah-Hartman, linux-kernel
Cc: Rafael J. Wysocki
The remove_nodes() open codes the list_for_each_safe_from().
Replace it by a generic macro.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v4: rebased on top of v5.13-rc2
drivers/base/devres.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 7970217191e0..db1f3137fc81 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -438,20 +438,16 @@ static int remove_nodes(struct device *dev,
struct list_head *first, struct list_head *end,
struct list_head *todo)
{
+ struct devres_node *node, *n;
int cnt = 0, nr_groups = 0;
- struct list_head *cur;
/* First pass - move normal devres entries to @todo and clear
* devres_group colors.
*/
- cur = first;
- while (cur != end) {
- struct devres_node *node;
+ node = list_entry(first, struct devres_node, entry);
+ list_for_each_entry_safe_from(node, n, end, entry) {
struct devres_group *grp;
- node = list_entry(cur, struct devres_node, entry);
- cur = cur->next;
-
grp = node_to_group(node);
if (grp) {
/* clear color of group markers in the first pass */
@@ -471,18 +467,14 @@ static int remove_nodes(struct device *dev,
/* Second pass - Scan groups and color them. A group gets
* color value of two iff the group is wholly contained in
- * [cur, end). That is, for a closed group, both opening and
- * closing markers should be in the range, while just the
+ * [current node, end). That is, for a closed group, both opening
+ * and closing markers should be in the range, while just the
* opening marker is enough for an open group.
*/
- cur = first;
- while (cur != end) {
- struct devres_node *node;
+ node = list_entry(first, struct devres_node, entry);
+ list_for_each_entry_safe_from(node, n, end, entry) {
struct devres_group *grp;
- node = list_entry(cur, struct devres_node, entry);
- cur = cur->next;
-
grp = node_to_group(node);
BUG_ON(!grp || list_empty(&grp->node[0].entry));
@@ -492,7 +484,7 @@ static int remove_nodes(struct device *dev,
BUG_ON(grp->color <= 0 || grp->color > 2);
if (grp->color == 2) {
- /* No need to update cur or end. The removed
+ /* No need to update current node or end. The removed
* nodes are always before both.
*/
list_move_tail(&grp->node[0].entry, todo);
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v4 4/4] devres: Enable trace events
2021-05-17 12:29 [PATCH v4 1/4] devres: Make locking straight forward in release_nodes() Andy Shevchenko
2021-05-17 12:29 ` [PATCH v4 2/4] devres: Use list_for_each_safe_from() in remove_nodes() Andy Shevchenko
2021-05-17 12:29 ` [PATCH v4 3/4] devres: No need to call remove_nodes() when there none present Andy Shevchenko
@ 2021-05-17 12:29 ` Andy Shevchenko
2021-05-26 11:53 ` [PATCH v4 1/4] devres: Make locking straight forward in release_nodes() Andy Shevchenko
3 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2021-05-17 12:29 UTC (permalink / raw)
To: Heikki Krogerus, Bartosz Golaszewski, Andy Shevchenko,
Greg Kroah-Hartman, 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>
---
v4: rebased on top of v5.13-rc2
drivers/base/Makefile | 3 +++
drivers/base/devres.c | 47 +++++++++++++++--------------------
drivers/base/trace.c | 10 ++++++++
drivers/base/trace.h | 56 ++++++++++++++++++++++++++++++++++++++++++
include/linux/device.h | 9 -------
5 files changed, 89 insertions(+), 36 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 dee48858663f..eaa9a5cd1db9 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.
@@ -134,26 +139,13 @@ static void replace_dr(struct device *dev,
list_replace(&old->entry, &new->entry);
}
-#ifdef CONFIG_DEBUG_DEVRES
-void * __devres_alloc_node(dr_release_t release, size_t size, gfp_t gfp, int nid,
- const char *name)
-{
- struct devres *dr;
-
- dr = alloc_dr(release, size, gfp | __GFP_ZERO, nid);
- if (unlikely(!dr))
- return NULL;
- set_node_dbginfo(&dr->node, name, size);
- return dr->data;
-}
-EXPORT_SYMBOL_GPL(__devres_alloc_node);
-#else
/**
- * devres_alloc_node - Allocate device resource data
+ * __devres_alloc_node - Allocate device resource data
* @release: Release function devres will be associated with
* @size: Allocation size
* @gfp: Allocation flags
* @nid: NUMA node
+ * @name: Name of the resource
*
* Allocate devres of @size bytes. The allocated area is zeroed, then
* associated with @release. The returned pointer can be passed to
@@ -162,17 +154,18 @@ EXPORT_SYMBOL_GPL(__devres_alloc_node);
* RETURNS:
* Pointer to allocated devres on success, NULL on failure.
*/
-void * devres_alloc_node(dr_release_t release, size_t size, gfp_t gfp, int nid)
+void *__devres_alloc_node(dr_release_t release, size_t size, gfp_t gfp, int nid,
+ const char *name)
{
struct devres *dr;
dr = alloc_dr(release, size, gfp | __GFP_ZERO, nid);
if (unlikely(!dr))
return NULL;
+ set_node_dbginfo(&dr->node, name, size);
return dr->data;
}
-EXPORT_SYMBOL_GPL(devres_alloc_node);
-#endif
+EXPORT_SYMBOL_GPL(__devres_alloc_node);
/**
* devres_for_each_res - Resource iterator
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>
diff --git a/include/linux/device.h b/include/linux/device.h
index 38a2071cf776..3769cce77e2c 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -165,21 +165,12 @@ void device_remove_bin_file(struct device *dev,
typedef void (*dr_release_t)(struct device *dev, void *res);
typedef int (*dr_match_t)(struct device *dev, void *res, void *match_data);
-#ifdef CONFIG_DEBUG_DEVRES
void *__devres_alloc_node(dr_release_t release, size_t size, gfp_t gfp,
int nid, const char *name) __malloc;
#define devres_alloc(release, size, gfp) \
__devres_alloc_node(release, size, gfp, NUMA_NO_NODE, #release)
#define devres_alloc_node(release, size, gfp, nid) \
__devres_alloc_node(release, size, gfp, nid, #release)
-#else
-void *devres_alloc_node(dr_release_t release, size_t size,
- gfp_t gfp, int nid) __malloc;
-static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
-{
- return devres_alloc_node(release, size, gfp, NUMA_NO_NODE);
-}
-#endif
void devres_for_each_res(struct device *dev, dr_release_t release,
dr_match_t match, void *match_data,
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread