On 06/20/19 06:33, Hans Verkuil wrote:
On 6/19/19 4:29 AM, Keith Pyle wrote:
On 06/18/19 02:16, Hans Verkuil wrote:
Hi Keith,
On 6/18/19 6:17 AM, Keith Pyle wrote:
We made the suggested change, compiled, installed, and rebooted. There was some progress - test 2 (turning the HD-PVR off) no longer produces a splat. Test 1 (start capture) and test 3 (run capture
and trigger HD-PVR to stop streaming) both still produce a traceback (see below). Test 3 also still results in the unkillable process.
Try the following patch. Test 2 was caused by locking when it shouldn't, test 3 was caused by not
locking when it should :-) and I think test 1 was caused by locking when it is not allowed.
Let me know if this works!
Regards,
Hans
Good news! With these patches, lockdep does not report any of the prior problems and the capture process does not deadlock for my test3.
There is one item I noted: hdpvr_read has the line
msec_to_jiffies(4000);
Oops!
that doesn't really do anything. This should be a 4 second sleep, based on our discussion back in 2014 (https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg75163.html), since the restart will
certainly fail unless the HD-PVR is given at least 3 seconds to reset after the stop.
I think a msleep(4000) at that point is solving only one use-case. I assume
the same problem will occur if you read() from the video device, then close()
it, re-open it and read() again, all within 4 seconds.
The real fix would be to store a timestamp (jiffies) when you stop streaming,
and in start_streaming check if there are less than 4 seconds between the last
stop and new start, and then sleep until 4 seconds have passed.
Is this something you can work on and provide a patch?
For now I'll post a patch fixing the deadlocks etc. so you can develop your
patch for this on top.
Regards,
Hans
I agree with your analysis that it would be better to have every
start_streaming make the check and sleep if needed. Yes, I can work on it.
Keith
