[PATCH v2 1/3] tlsf: dump stack on assertion failure

[PATCH v2 1/3] tlsf: dump stack on assertion failure

[Ahmad Fatoum]

We only support AddressSanitizer on ARM, ARM64 and sandbox. For other
platforms TLSF assertions may detect some nconsistency, like some double
frees. Make the reports more useful by dumping stack in that case.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2: no change
---
 include/tlsf.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/tlsf.h b/include/tlsf.h
index 7015de0eb525..161176d5ac84 100644
--- a/include/tlsf.h
+++ b/include/tlsf.h
@@ -42,9 +42,12 @@
 extern "C" {
 #endif
 
+#include <printk.h>
+
 #define tlsf_assert(expr) do {                              \
         if (unlikely(!(expr))) {                            \
                 printf(#expr "%s %d\n", __FILE__, __LINE__); \
+                dump_stack();                               \
         }                                                   \
 } while (0)
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 2/3] driver: have CONFIG_DEBUG_PROBES report device unbind as well

[Ahmad Fatoum]

This aligns it with DEBUG_INITCALLS, which also traces exitcalls.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2: no change
---
 common/Kconfig        | 10 ++++++++--
 drivers/base/driver.c |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 482faea933a6..060e21d9fedf 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1524,9 +1524,15 @@ config DEBUG_INITCALLS
 	  If enabled this will print initcall traces.
 
 config DEBUG_PROBES
-	bool "Trace driver probes"
+	bool "Trace driver probes/removes"
 	help
-	  If enabled this will print driver probe traces.
+	  If enabled this will log driver probe and remove traces. If DEBUG_LL is enabled,
+	  probes will be printed even before registering consoles. If it's disabled, they
+	  will be collected in the log and written out once a console is active.
+
+	  Removes are written to the log and will be printed as long as consoles exist.
+	  Most consoles do not implement a remove callback to remain operable until
+	  the very end. Consoles using DMA, however, must be removed.
 
 config PBL_BREAK
 	bool "Execute software break on pbl start"
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index dd965eb165ee..bb07e96dcaf4 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -502,11 +502,15 @@ EXPORT_SYMBOL_GPL(dev_set_name);
 static void devices_shutdown(void)
 {
 	struct device_d *dev;
+	int depth = 0;
 
 	list_for_each_entry(dev, &active, active) {
 		if (dev->bus->remove) {
+			depth++;
+			pr_report_probe("%*sremove-> %s\n", depth * 4, "", dev_name(dev));
 			dev->bus->remove(dev);
 			dev->driver = NULL;
+			depth--;
 		}
 	}
 }
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 3/3] commands: add new devunbind debugging command

[Ahmad Fatoum]

Memory corruption around device removal may go unnoticed, because
barebox is shutting down anyway and doing no new allocations.

Add a new devunbind command that should help with debugging such issues
by allowing selective unbinding and removal of devices.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - updated Kconfig discription with -l option
  - fixed too early exit after unregistering first device (Sascha)
  - report skipping of missing device
---
 commands/Kconfig      | 12 +++++++
 commands/Makefile     |  1 +
 commands/devunbind.c  | 79 +++++++++++++++++++++++++++++++++++++++++++
 drivers/base/driver.c |  7 ++--
 include/driver.h      |  4 +++
 5 files changed, 100 insertions(+), 3 deletions(-)
 create mode 100644 commands/devunbind.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 5506d1b8f07c..ba8ca5cdebce 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -75,6 +75,18 @@ config CMD_DEVINFO
 	  If called with a device path being the argument, devinfo shows more
 	  default information about this device and its parameters.
 
+config CMD_DEVUNBIND
+	tristate
+	prompt "devunbind"
+	help
+	  Debugging aid to unbind device(s) from driver at runtime
+
+	  devunbind [-fl] DEVICES..
+
+	  Options:
+		-f   unbind driver and force removal of device and children
+		-l   list remove callbacks in shutdown order
+
 config CMD_DMESG
 	tristate
 	prompt "dmesg"
diff --git a/commands/Makefile b/commands/Makefile
index c1a060da5204..db78d0b877f6 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_CMD_MIITOOL)	+= miitool.o
 obj-$(CONFIG_CMD_DETECT)	+= detect.o
 obj-$(CONFIG_CMD_BOOT)		+= boot.o
 obj-$(CONFIG_CMD_DEVINFO)	+= devinfo.o
+obj-$(CONFIG_CMD_DEVUNBIND)	+= devunbind.o
 obj-$(CONFIG_CMD_DRVINFO)	+= drvinfo.o
 obj-$(CONFIG_CMD_READF)		+= readf.o
 obj-$(CONFIG_CMD_MENUTREE)	+= menutree.o
diff --git a/commands/devunbind.c b/commands/devunbind.c
new file mode 100644
index 000000000000..3f9cd7b849c8
--- /dev/null
+++ b/commands/devunbind.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// SPDX-FileCopyrightText: © 2021 Ahmad Fatoum <a.fatoum@pengutronix.de>, Pengutronix
+
+#include <command.h>
+#include <common.h>
+#include <complete.h>
+#include <driver.h>
+#include <getopt.h>
+
+static int do_devunbind(int argc, char *argv[])
+{
+	bool unregister = false;
+	struct device_d *dev;
+	int ret = COMMAND_SUCCESS, i, opt;
+
+	while ((opt = getopt(argc, argv, "fl")) > 0) {
+		switch (opt) {
+		case 'f':
+			unregister = true;
+			break;
+		case 'l':
+			list_for_each_entry(dev, &active_device_list, active) {
+				BUG_ON(!dev->driver);
+				if (dev->bus->remove)
+					printf("%pS(%s, %s)\n", dev->bus->remove,
+					       dev->driver->name, dev_name(dev));
+			}
+			return 0;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (!argv[optind])
+		return COMMAND_ERROR_USAGE;
+
+	for (i = optind; i < argc; i++) {
+		dev = get_device_by_name(argv[i]);
+		if (!dev) {
+			printf("skipping missing %s\n", argv[i]);
+			ret = -ENODEV;
+			continue;
+		}
+
+		if (unregister) {
+			unregister_device(dev);
+			continue;
+		}
+
+		if (!dev->driver || !dev->bus->remove) {
+			printf("skipping unbound %s\n", argv[i]);
+			ret = COMMAND_ERROR;
+			continue;
+		}
+
+		dev->bus->remove(dev);
+		dev->driver = NULL;
+		list_del(&dev->active);
+	}
+
+	return ret;
+}
+
+BAREBOX_CMD_HELP_START(devunbind)
+BAREBOX_CMD_HELP_TEXT("Debugging aid to unbind device from driver at runtime")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-f",  "unbind driver and force removal of device and children")
+BAREBOX_CMD_HELP_OPT ("-l",  "list remove callbacks in shutdown order")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(devunbind)
+	.cmd		= do_devunbind,
+	BAREBOX_CMD_DESC("unbind device(s) from driver")
+	BAREBOX_CMD_OPTS("[-fl] DEVICES..")
+	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
+	BAREBOX_CMD_HELP(cmd_devunbind_help)
+	BAREBOX_CMD_COMPLETE(device_complete)
+BAREBOX_CMD_END
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index bb07e96dcaf4..f54f4d0b3746 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -41,7 +41,8 @@ EXPORT_SYMBOL(device_list);
 LIST_HEAD(driver_list);
 EXPORT_SYMBOL(driver_list);
 
-static LIST_HEAD(active);
+LIST_HEAD(active_device_list);
+EXPORT_SYMBOL(active_device_list);
 static LIST_HEAD(deferred);
 
 struct device_d *get_device_by_name(const char *name)
@@ -91,7 +92,7 @@ int device_probe(struct device_d *dev)
 	pinctrl_select_state_default(dev);
 	of_clk_set_defaults(dev->device_node, false);
 
-	list_add(&dev->active, &active);
+	list_add(&dev->active, &active_device_list);
 
 	ret = dev->bus->probe(dev);
 	if (ret == 0)
@@ -504,7 +505,7 @@ static void devices_shutdown(void)
 	struct device_d *dev;
 	int depth = 0;
 
-	list_for_each_entry(dev, &active, active) {
+	list_for_each_entry(dev, &active_device_list, active) {
 		if (dev->bus->remove) {
 			depth++;
 			pr_report_probe("%*sremove-> %s\n", depth * 4, "", dev_name(dev));
diff --git a/include/driver.h b/include/driver.h
index 4f6d40e17c14..1215a2d57ab3 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -328,6 +328,10 @@ extern struct list_head device_list;
  */
 extern struct list_head driver_list;
 
+/* linear list over all active devices
+ */
+extern struct list_head active_device_list;
+
 /* Iterate over all devices
  */
 #define for_each_device(dev) list_for_each_entry(dev, &device_list, list)
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/3] tlsf: dump stack on assertion failure

[Sascha Hauer]

On Thu, Jan 13, 2022 at 05:04:12PM +0100, Ahmad Fatoum wrote:
> We only support AddressSanitizer on ARM, ARM64 and sandbox. For other
> platforms TLSF assertions may detect some nconsistency, like some double
> frees. Make the reports more useful by dumping stack in that case.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2: no change
> ---
>  include/tlsf.h | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox