All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antti Palosaari <crope@iki.fi>
To: David Waring <davidjw@rd.bbc.co.uk>
Cc: linux-media@vger.kernel.org
Subject: Re: recursive locking problem
Date: Tue, 13 Sep 2011 23:59:06 +0300	[thread overview]
Message-ID: <4E6FC41A.5030803@iki.fi> (raw)
In-Reply-To: <4E69EE5E.8080605@rd.bbc.co.uk>

On 09/09/2011 01:45 PM, David Waring wrote:
> On 08/09/11 17:34, Antti Palosaari wrote:
>> [snip]
>>
>> Is there any lock can do recursive locking but unlock frees all locks?
>>
>> Like that:
>> gate_open
>> +gate_open
>> +gate_close
>> == lock is free
>>
>> AFAIK mutex can do only simple lock() + unlock(). Semaphore can do
>> recursive locking, like lock() + lock() + unlock() + unlock(). But how I
>> can do lock() + lock() + unlock() == free.
>>
> Antti,
>
> It's a very bad idea to try and use a mutex like that. The number of
> locks and unlocks must be balanced otherwise you risk accessing
> variables without a lock.
>
> Consider:
>
> static struct mutex foo_mutex;
> static int foo=3;
>
> void a() {
>    mutex_lock(&foo_mutex);
>    if (foo<5) foo++;
>    b();
>    foo--; /*<<<  still need lock here */
>    mutex_unlock(&foo_mutex);
> }
>
> void b() {
>    mutex_lock(&foo_mutex);
>    if (foo>6) foo=(foo>>1);
>    mutex_unlock(&foo_mutex);
> }
>
> Note: this assumes mutex_lock will allow the same thread get multiple
> locks as you would like (which it doesn't).
>
> As pointed out in the code, when a() is called, you still need the lock
> for accesses to foo after the call to b() that also requires the lock.
> If we used the locks in the way you propose then foo would be accessed
> without a lock.
>
> To code properly for cases like these I usually use a wrapper functions
> to acquire the lock and call a thread unsafe version (i.e. doesn't use
> locks) of the function that only uses other thread unsafe functions. e.g.
>
> void a() {
>    mutex_lock(&foo_mutex);
>    __a_thr_unsafe();
>    mutex_unlock(&foo_mutex);
> }
>
> void b() {
>    mutex_lock(&foo_mutex);
>    __b_thr_unsafe();
>    mutex_unlock(&foo_mutex);
> }
>
> static void __a_thr_unsafe() {
>    if (foo<5) foo++;
>    __b_thr_unsafe();
>    foo--;
> }
>
> static void __b_thr_unsafe() {
>    if (foo>6) foo=(foo>>1);
> }
>
> This way a call to a() or b() will acquire the lock once for that
> thread, perform all actions and then release the lock. The mutex is
> handled properly.
>
> Can you restructure the code so that you don't need multiple locks?

Thank you for very long and detailed reply with examples :)

I need lock for hardware access. Single I2C-adapter have two I2C-clients 
that have same I2C-address in same bus. There is gate (demod I2C-gate) 
logic that is used to select desired tuner. See that:
http://palosaari.fi/linux/v4l-dvb/controlling_tuner_af9015_dual_demod.txt

You can never know surely how tuner drivers calls to open or close gate, 
very commonly there is situations where multiple close or open happens. 
That's why lock/unlock is problematic.

.i2c_gate_ctrl() is demod driver callback (struct dvb_frontend_ops) 
which controls gate that gate. That callback is always called from tuner 
driver when gate is needed to open or close.

regards
Antti

-- 
http://palosaari.fi/

  reply	other threads:[~2011-09-13 20:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-08 16:34 recursive locking problem Antti Palosaari
2011-09-09  7:51 ` Hans Petter Selasky
2011-09-09 10:45 ` David Waring
2011-09-13 20:59   ` Antti Palosaari [this message]
2011-09-13 21:34     ` Steve Kerrison
2011-09-13 21:58       ` Steven Toth
2011-09-13 22:10         ` Devin Heitmueller
2011-09-13 22:19           ` Steven Toth
2011-09-13 22:01       ` Devin Heitmueller
2011-09-09 11:46 ` Daniel Glöckner
2011-09-14  1:03   ` Antti Palosaari
2011-09-14  6:19     ` Daniel Glöckner
2011-09-14 10:45       ` Antti Palosaari
2011-09-14 12:22         ` Daniel Glöckner

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=4E6FC41A.5030803@iki.fi \
    --to=crope@iki.fi \
    --cc=davidjw@rd.bbc.co.uk \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.