From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Devin Heitmueller <dheitmueller@kernellabs.com>
Cc: Dmitri Belimov <d.belimov@gmail.com>,
Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH] xc5000, fix fw upload crash
Date: Fri, 20 May 2011 19:22:29 -0300 [thread overview]
Message-ID: <4DD6E9A5.10406@redhat.com> (raw)
In-Reply-To: <BANLkTi=otyZxEof89KbDbLXCLz4XsT=5ww@mail.gmail.com>
Em 20-05-2011 10:04, Devin Heitmueller escreveu:
> On Friday, May 20, 2011, Dmitri Belimov <d.belimov@gmail.com> wrote:
>> Hi Devin
>>
>> snip
>>
>>> NACK!
>>>
>>> I don't think this patch is correct. Concurrency problems are
>>> expected to be handled in the upper layers, as there are usually much
>>> more significant problems than just this case. For example, if this
>>> is a race between V4L2 and DVB, it is the responsibility of bridge
>>> driver to provide proper locking.
>>>
...
>>
>> I see two different way add mutex to function where firmware is loaded or to
>> xc5000_set_analog_params
>>
>> Both of this is working I already test it.
>>
>> What you think about it??
...
>> [ 110.010686] [<f81cb6d8>] ? set_mode_freq+0xe4/0xff [tuner]
>> [ 110.010689] [<f81cb8d4>] ? tuner_s_std+0x26/0x5aa [tuner]
>> [ 110.010692] [<f81cb8ae>] ? tuner_s_std+0x0/0x5aa [tuner]
Hmm... this is probably caused by the BKL removal patches.
Basically, tuner_s_std is being called without holding dev->lock. The fix is
simple, but requires some care: we need either to convert saa7134 to the
v4l2 core support (probably not an easy task) or to review all places where
dev->lock should be used, e. g. at (almost all) ioctls, and at the other
file ops (open, close, mmap, etc). This driver is complex, due to the mpeg
optional module used on some devices. So, maybe the in-core locking schema
is not the proper way to fix it.
Cheers,
Mauro.
next prev parent reply other threads:[~2011-05-20 22:22 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-17 4:23 [PATCH] xc5000, fix fw upload crash Dmitri Belimov
2011-05-18 1:27 ` Devin Heitmueller
2011-05-20 4:46 ` Dmitri Belimov
2011-05-20 13:04 ` Devin Heitmueller
2011-05-20 22:22 ` Mauro Carvalho Chehab [this message]
2011-05-25 6:40 ` Dmitri Belimov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DD6E9A5.10406@redhat.com \
--to=mchehab@redhat.com \
--cc=d.belimov@gmail.com \
--cc=dheitmueller@kernellabs.com \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox