On 01/04/2017 12:20 AM, Bryan O'Donoghue wrote:
On 03/01/17 17:57, Alexandre Bailon wrote:
Exiting the thread let loopback in incorrect state: nothing will work because the thread stopped but sysfs / debugfs files are still there.
actually... reading your email again
why does the thread need to exit because gb_pm_runtime_get_sync() failed ?
Can't we just print some sort of warning message and continue ?
diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c index 3184dd3..dfdcea3 100644 --- a/drivers/staging/greybus/loopback.c +++ b/drivers/staging/greybus/loopback.c @@ -993,7 +993,7 @@ static int gb_loopback_fn(void *data) kthread_should_stop()); ret = gb_pm_runtime_get_sync(bundle); if (ret)
return ret;
dev_warn(&bundle->dev, "broken - ohwell\n"); }
if (kthread_should_stop())I'm volunteer to fix the issue but I'm not sure how to do it, and may be there is other better solutions I might have missed. So, all suggestion are welcome.
You made a valid point about sysfs entries - these should only be cleaned up in the module exit path and a do_exit() as I suggested in my previous email would (I think anyway) fix the error you reported on rmmod but would still leave /sysfs entries dangling around between the time of the do_exit() and the rmmod (if it ever happened) which would be wrong.
So - can't we just ignore gb_pm_runtime_get_sync() if it returns an error ?
I think it's a good solution. In addition of avoiding the kthread issue, it will also let the thread try to do a transfer which will not work and rise an error to the user space application.
Trying to exit the thread and cleanup the associated /sysfs entries would be a dogs dinner of a solution IMO.
bod