On Fri, Dec 9, 2011 at 8:37 PM, Andreas Oberritter <o...@linuxtv.org> wrote:
> On 10.12.2011 00:43, Mauro Carvalho Chehab wrote:
>> On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
>>> On 09-12-2011 20:33, Devin Heitmueller wrote:
>>>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>>>> <mche...@redhat.com> wrote:
>>>>>> Could someone explain reason for that?
>>>>>
>>>>>
>>>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>>>> is opened with O_NONBLOCK.
>>>>
>>>> Are you doing the drx-k firmware load on dvb_init()? That could
>>>> easily take 4 seconds.
>>>
>>> No. The firmware were opened previously.
>>
>> Maybe the delay is due to this part of dvb_frontend.c:
>>
>> static int dvb_mfe_wait_time = 5;
>> ...
>> int mferetry = (dvb_mfe_wait_time << 1);
>>
>> mutex_unlock (&adapter->mfe_lock);
>> while (mferetry-- && (mfedev->users != -1 ||
>> mfepriv->thread != NULL)) {
>> if(msleep_interruptible(500)) {
>> if(signal_pending(current))
>> return -EINTR;
>> }
>> }
>
> I haven't looked at the mfe code, but in case it's waiting for the
> frontend thread to exit, there's a problem that causes the thread
> not to exit immediately. Here's a patch that's been sitting in my
> queue for a while:
>
> ---
>
> Signed-off-by: Andreas Oberritter <o...@linuxtv.org>
>
> diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> index 7784d74..6823c2b 100644
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c 2011-09-07
> 12:32:24.000000000 +0200
> +++ a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c 2011-09-13
> 15:55:48.865742791 +0200
> @@ -514,7 +514,7 @@
> return 1;
>
> if (fepriv->dvbdev->writers == 1)
> - if (time_after(jiffies, fepriv->release_jiffies +
> + if (time_after_eq(jiffies, fepriv->release_jiffies +
> dvb_shutdown_timeout * HZ))
> return 1;
>
> @@ -2070,12 +2070,15 @@
>
> dprintk ("%s\n", __func__);
>
> - if ((file->f_flags & O_ACCMODE) != O_RDONLY)
> + if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
> fepriv->release_jiffies = jiffies;
> + mb();
> + }
>
> ret = dvb_generic_release (inode, file);
>
> if (dvbdev->users == -1) {
> + wake_up(&fepriv->wait_queue);
> if (fepriv->exit != DVB_FE_NO_EXIT) {
> fops_put(file->f_op);
> file->f_op = NULL;
This patch needs to have a much better explanation of exactly what it
does and what problem it solves. We have a history of race conditions
in dvb_frontend.c, and it's patches like this with virtually no
details just makes it worse.
I'm not arguing the actual merits of the code change - it *may* be
correct. But without the appropriate background there is no real way
of knowing...
Mauro, this patch should be NACK'd and resubmitted with a detailed
explanation of the current behavior, what the problem is, and how the
code changes proposed solve that problem.
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
