[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7361d00d-9cfe-3e4a-6199-524d37d53bd0@gmail.com>
Date: Mon, 6 Apr 2020 23:54:46 +0300
From: Dmitry Osipenko <digetx@...il.com>
To: Sowjanya Komatineni <skomatineni@...dia.com>,
thierry.reding@...il.com, jonathanh@...dia.com, frankc@...dia.com,
hverkuil@...all.nl, sakari.ailus@....fi, helen.koike@...labora.com
Cc: sboyd@...nel.org, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v6 6/9] media: tegra: Add Tegra210 Video input driver
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.
Powered by blists - more mailing lists