On Sun, Jan 08, 2017 at 03:27:19PM +0000, Bryan O'Donoghue wrote:
Doing an rmmod gb_loopback will cause an oops with the following backtrace.
Call Trace: debugfs_remove+0xaf/0x180 gb_loopback_disconnect+0x36/0x160 [gb_loopback] greybus_remove+0x87/0x1a0 [greybus] device_release_driver_internal+0x14a/0x200 driver_detach+0x39/0x60 bus_remove_driver+0x47/0xa0 driver_unregister+0x27/0x50 greybus_deregister_driver+0xd/0x10 [greybus] loopback_exit+0x1c/0x36 [gb_loopback] SyS_delete_module+0x170/0x1b0 entry_SYSCALL_64_fastpath+0x13/0x93
This happens because gb_dev.root is removed prior to greybus_deregister_driver which itself will trigger gb_loopback_disconnect to run - leading to the parent of gb->file being NULL.
Typically up to this point when tearing down greybus modules we have removed gb_es2 prior to removing gb_loopback. In this case gb_es2 will run subordinate greybus->disconnect() routines. A subsequent rmmod of gb_loopback then will not re-run gb_loopback_disconnect().
On project Ara the second module removal flow was the only practiced flow hence it's only now in gbsim where we have more freedom to rmmod that this bug is apparent. The fix itself is pretty straight-forward.
There was nothing preventing you from unloading bundle-drivers while keeping the host-device driver (e.g. gb_es2) loaded on Project Ara, even if this typically wasn't done, yes.
drivers/staging/greybus/loopback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 88d4f5c..8f3eb33 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -1260,9 +1260,9 @@ module_init(loopback_init); static void __exit loopback_exit(void) {
- debugfs_remove_recursive(gb_dev.root); greybus_deregister(&gb_loopback_driver); class_unregister(&loopback_class);
- debugfs_remove_recursive(gb_dev.root);
This is just a plain bug, and in fact, you should only be using debugfs_remove here, as any bundle-specific files should have been cleaned up as part of disconnect.
ida_destroy(&loopback_ida); } module_exit(loopback_exit);
Thanks, Johan