Re: [RFC PATCH v9 6/9] media: tegra: Add Tegra210 Video input driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/23/20 7:09 PM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


24.04.2020 04:08, Sowjanya Komatineni пишет:
On 4/23/20 5:51 PM, Sowjanya Komatineni wrote:
On 4/23/20 5:42 PM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


24.04.2020 02:50, Sowjanya Komatineni пишет:
On 4/23/20 4:25 PM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


24.04.2020 02:20, Sowjanya Komatineni пишет:
On 4/23/20 4:19 PM, Sowjanya Komatineni wrote:
On 4/23/20 4:16 PM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


22.04.2020 09:18, Sowjanya Komatineni пишет:
+static int chan_capture_kthread_start(void *data)
+{
+     struct tegra_vi_channel *chan = data;
+     struct tegra_channel_buffer *buf;
+     int err = 0;
+
+     set_freezable();
+
+     while (1) {
+             try_to_freeze();
+
+ wait_event_interruptible(chan->start_wait,
+ !list_empty(&chan->capture) ||
+ kthread_should_stop());
+
+             if (kthread_should_stop())
+                     break;
+
+             /*
+              * Source is not streaming if error is non-zero.
+              * So, do not dequeue buffers on capture error.
+              */
+             if (err)
+                     continue;
This will result in an endless loop, I suppose it wasn't the
intention.
no it will not. on error we report vb2_queue_error which will do
streaming stop request.

So thread will be stopped on streaming stop request thru kthread
stop
signal
To be clear on error it reports vb2 queue error and waits for stop
streaming to happen
If thread should exit on error, then it should do it on the actual
error. Otherwise it looks very error-prone.
When v4l2 drivers indicate fatal error through vb2_queue_error, queue
error flag  is set and wakes up all processes waiting on queue along
with polling reporting  EPOLLERR and also reporting error for queuing
and dequeuing buffers. Stream stop will surely happen which stops the
thread.
This doesn't explain what is the point of continuing to loop instead of
exiting immediately on error.
We are using 2 threads and when capture start error happens, we can
stop capture_start thread immediately but capture_finish thread will
still run for any outstanding buffers.

So, as it makes no diff stopping both threads during stream stop which
will definitely happen on error and when we don't dequeue buffers

Also there will be an issue if we break on error immediately during
stop_streaming -> kthread_stop()

As stop streaming can happen any time, we do kthread_stop and in case of
error if we stop thread and on stop streaming kthread_stop might crash
as kthread_stop can only be called on running thread

This a better explanation, but still it's not good that there could be a
busy loop within the thread.

Should be better to sleep if error is set.

wait_event_interruptible(chan->start_wait,
                         (!err && !list_empty(&chan->capture)) ||
                         kthread_should_stop());
ok, will add err to wait condition to let it sleep




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux