On January 9, 2017 11:19:09 AM GMT+00:00, Johan Hovold <johan@xxxxxxxxxx> wrote:
>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.
The greybus operations themselves do report an error subsequent to the user-space tool.
>Kill the thread, or somehow report a test failure and wait for user
>space to retry.
Killing the thread would be messy, however yes I take your general point.
It should be possible to use the gb_pm_runtime_get_sync as a determinant and cease further operations.
I spin that into a v2 this evening.
>
>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
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
