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

[PATCH 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>
---
 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 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>
---
 common/Kconfig        | 10 ++++++++--
 drivers/base/driver.c |  4 ++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index d0055e2d2182..802bd9bfbb2e 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1503,9 +1503,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 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>
---
 commands/Kconfig      | 12 +++++++
 commands/Makefile     |  1 +
 commands/devunbind.c  | 74 +++++++++++++++++++++++++++++++++++++++++++
 drivers/base/driver.c |  7 ++--
 include/driver.h      |  4 +++
 5 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100644 commands/devunbind.c

diff --git a/commands/Kconfig b/commands/Kconfig
index e2c36949347e..9abd97271952 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -68,6 +68,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
+	default y
+	prompt "devunbind"
+	help
+	  Debugging aid to unbind device from driver at runtime
+
+	  devunbind [-f] DEVICE
+
+	  Options:
+	    -f   unbind driver and force removal of device and children
+
 config CMD_DMESG
 	tristate
 	prompt "dmesg"
diff --git a/commands/Makefile b/commands/Makefile
index 0b7c1563b534..875826743ffe 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..4bebb27e8e68
--- /dev/null
+++ b/commands/devunbind.c
@@ -0,0 +1,74 @@
+// 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)
+			return -ENODEV;
+
+		if (unregister)
+			return unregister_device(dev);
+
+		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 3/3] commands: add new devunbind debugging command

[Sascha Hauer]

On Sat, Jan 08, 2022 at 06:14:26PM +0100, Ahmad Fatoum wrote:
> 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>
> ---
>  commands/Kconfig      | 12 +++++++
>  commands/Makefile     |  1 +
>  commands/devunbind.c  | 74 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/base/driver.c |  7 ++--
>  include/driver.h      |  4 +++
>  5 files changed, 95 insertions(+), 3 deletions(-)
>  create mode 100644 commands/devunbind.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index e2c36949347e..9abd97271952 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -68,6 +68,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
> +	default y
> +	prompt "devunbind"
> +	help
> +	  Debugging aid to unbind device from driver at runtime
> +
> +	  devunbind [-f] DEVICE
> +
> +	  Options:
> +	    -f   unbind driver and force removal of device and children
> +
>  config CMD_DMESG
>  	tristate
>  	prompt "dmesg"
> diff --git a/commands/Makefile b/commands/Makefile
> index 0b7c1563b534..875826743ffe 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..4bebb27e8e68
> --- /dev/null
> +++ b/commands/devunbind.c
> @@ -0,0 +1,74 @@
> +// 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)
> +			return -ENODEV;
> +
> +		if (unregister)
> +			return unregister_device(dev);

Shouldn't you continue the loop here?

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

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

[Ahmad Fatoum]

On 10.01.22 09:56, Sascha Hauer wrote:
> On Sat, Jan 08, 2022 at 06:14:26PM +0100, Ahmad Fatoum wrote:
>> 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>
>> ---
>>  commands/Kconfig      | 12 +++++++
>>  commands/Makefile     |  1 +
>>  commands/devunbind.c  | 74 +++++++++++++++++++++++++++++++++++++++++++
>>  drivers/base/driver.c |  7 ++--
>>  include/driver.h      |  4 +++
>>  5 files changed, 95 insertions(+), 3 deletions(-)
>>  create mode 100644 commands/devunbind.c
>>
>> diff --git a/commands/Kconfig b/commands/Kconfig
>> index e2c36949347e..9abd97271952 100644
>> --- a/commands/Kconfig
>> +++ b/commands/Kconfig
>> @@ -68,6 +68,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
>> +	default y
>> +	prompt "devunbind"
>> +	help
>> +	  Debugging aid to unbind device from driver at runtime
>> +
>> +	  devunbind [-f] DEVICE
>> +
>> +	  Options:
>> +	    -f   unbind driver and force removal of device and children
>> +
>>  config CMD_DMESG
>>  	tristate
>>  	prompt "dmesg"
>> diff --git a/commands/Makefile b/commands/Makefile
>> index 0b7c1563b534..875826743ffe 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..4bebb27e8e68
>> --- /dev/null
>> +++ b/commands/devunbind.c
>> @@ -0,0 +1,74 @@
>> +// 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)
>> +			return -ENODEV;
>> +
>> +		if (unregister)
>> +			return unregister_device(dev);
> 
> Shouldn't you continue the loop here?

Yes. Fixed in v2.

> 
> 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