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
_______________________________________________
greybus-dev mailing list
greybus-dev@xxxxxxxxxxxxxxxx
https://lists.linaro.org/mailman/listinfo/greybus-dev
|