On Sun, Jan 08, 2017 at 03:27:18PM +0000, Bryan O'Donoghue wrote:
> commit e854ff58ed70 ("greybus: loopback: add runtime pm support")
> introduces pm runtime support to the loopback code. If a
> gb_pm_runtime_get_sync() fails, currently we immediately return from the
> loopback thread.
>
> Alexandre reports that later on, subsequent to the afore mentioned failure,
> doing an rmmod will trigger a kthread_stop() which will will generate a
> fault. The fault results from dereferencing gb->task in
> gb_loopback_disconnect().
>
> One way to fix that is to have the loopback thread do_exit() instead of
> simply returning on gb_pm_runtime_get_sync() failure - however this will
> leave dangling sysfs entries with no loopback thread to take action based
> on the sysfs inputs.
>
> This patch fixes the fault by ignoring the gb_pm_runtime_get_sync() result
> code, this will allow only gb_loopback_disconnect() to terminate the
> loopback thread. Fix validated in gbsim.
No, you must check the return for resume errors, and not attempt any
further I/O in case of failure.
Kill the thread, or somehow report a test failure and wait for user
space to retry.
This is not directly related to the sysfs entries, they belong to the
bundle and should stay while the device is present. As mentioned before,
the sysfs-interface is not the right interface for this type of device,
but that's a different story.
> Next steps on this driver entail changing the relationship between /sysfs
> and the loopback thread - probably making the loopback thread dynamically
> started/stopped - as opposed to the pre-allocation model currently used
> but, those changes are for a future series to address.
You might get away with using a work struct scheduled from the current
sysfs-interface, but long term this should probably be shifted over to
using a character-device interface.
Thanks,
Johan
_______________________________________________
greybus-dev mailing list
greybus-dev@xxxxxxxxxxxxxxxx
https://lists.linaro.org/mailman/listinfo/greybus-dev
|