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

[Sowjanya Komatineni]

On 4/6/20 1:54 PM, Dmitry Osipenko wrote:
> External email: Use caution opening links or attachments
>
>
> 06.04.2020 23:38, Sowjanya Komatineni ÐÐÑÐÑ:
> > On 4/6/20 1:37 PM, Dmitry Osipenko wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > 06.04.2020 23:20, Sowjanya Komatineni ÐÐÑÐÑ:
> > > > On 4/6/20 1:02 PM, Dmitry Osipenko wrote:
> > > > > External email: Use caution opening links or attachments
> > > > >
> > > > >
> > > > > 04.04.2020 04:25, Sowjanya Komatineni ÐÐÑÐÑ:
> > > > > ...
> > > > > > +static int chan_capture_kthread_start(void *data)
> > > > > > +{
> > > > > > +     struct tegra_vi_channel *chan = data;
> > > > > > +     struct tegra_channel_buffer *buf;
> > > > > > +     int err = 0;
> > > > > > +     int caps_inflight;
> > > > > > +
> > > > > > +     set_freezable();
> > > > > > +
> > > > > > +     while (1) {
> > > > > > +             try_to_freeze();
> > > > > > +
> > > > > > +             wait_event_interruptible(chan->start_wait,
> > > > > > +                                      !list_empty(&chan->capture) ||
> > > > > > +                                      kthread_should_stop());
> > > > > Is it really okay that list_empty() isn't protected with a lock?
> > > > >
> > > > > Why wait_event is "interruptible"?
> > > > To allow it to sleep until wakeup on thread it to avoid constant
> > > > checking for condition even when no buffers are ready, basically to
> > > > prevent blocking.
> > > So the "interrupt" is for getting event about kthread_should_stop(),
> > > correct?
> > also to prevent blocking and to let is sleep and wakeup based on wait
> > queue to evaluate condition to proceed with the task
> This looks suspicious, the comment to wait_event_interruptible() says
> that it will return ERESTARTSYS if signal is recieved..
>
> Does this mean that I can send signal from userspace to wake it up?
>
> The "interruptible" part looks wrong to me.

We are not checking for wait_event_interruptible to handle case when it 
returns ERESTARTSYS.

So, signals sent from user space are ignore and we check if when wakeup 
happens if kthread_stop has requested to stop thread.