Re: rme9652: possible deadlock

[Abramo Bagnara <abramo.bagnara@libero.it>]

Jaroslav Kysela ha scritto:
> On Mon, 28 Apr 2003, Abramo Bagnara wrote:
> 
> 
>>Jaroslav Kysela ha scritto:
>>
>>>>CPU1:
>>>>
>>>>read_lock(link_manager);	read_lock(link_manager);
>>>>spin_lock(s1);			spin_lock(s2);
>>>>if (stream_in_group)		if (stream_in_group)
>>>>	spin_lock(group);		spin_lock(group); /* Waiting */
>>>>spin_lock(s2); /* BOOM! */
>>>>
>>>>I guess that 3rd version will be the one I propose ;-)
>>>
>>>
>>>Yep, it is in CVS now ;-) Except I don't use trylock...
>>>
>>>read_lock(link_manager);
>>>spin_lock(stream);
>>>....
>>>if (stream_in_group) {
>>>    spin_lock(group);
>>>    spin_unlock(stream);
>>>}
>>>....
>>>link_for_each(s) {	/* note that streams are locked in defined order now */
>>>    if (stream_in_group)
>>>        spin_lock(s);
>>>}
>>>....
>>>list_for_each(s) {
>>>    if (stream_in_group)
>>>        spin_unlock(s);
>>>}
>>>....
>>>if (stream_in_group) {
>>>    spin_lock(stream);
>>>    spin_unlock(group);
>>>}
>>>....
>>>spin_unlock(stream);
>>>read_unlock(link_manager);
>>
>>CPU1:				CPU2:
>>read_lock(link_manager);	read_lock(link_manager);
>>spin_lock(s1);			spin_lock(s2);
>>spin_lock(group);		spin_lock(group); // waiting
>>spin_unlock(s1);
>>//inside list_for_each
>>spin_lock(s1);
>>spin_lock(s2); // BOOM!
>>
>>I fear you cannot avoid trylock ;-)
> 
> 
> And this scenario:
> 
> read_lock(link_manager);
> spin_lock(s1);
> if (stream_in_lock) {
>   spin_unlock(s1);
>   spin_lock(group);
> }

It's unsafe (just like my solution) but without a way to knows that 
stream state might be changed (unlike my solution).

An example: on enter of snd_pcm_update_hw_ptr_interrupt stream state is 
RUNNING but on exit it might be XRUN.

The only way to solve all that is to accurately invalid any implicitly 
cached info across call to functions that promoting a single stream lock 
to a group lock have waited on group lock.

Of course you might enlarge this criteria to "across call to functions 
that might promote a single stream lock to a group lock". In this case 
your solution is fine too.

As a side note I've noted that almost all calls to SND_PCM_ACTION take 
the single lock just before. This is definitely a non sense: taking the 
group lock at 1st is the right solution. This imply to have two 
SND_PCM_ACTION: one for already locked groups and another for lock 
promotion.

-- 
Abramo Bagnara                       mailto:abramo.bagnara@libero.it

Opera Unica                          Phone: +39.546.656023
Via Emilia Interna, 140
48014 Castel Bolognese (RA) - Italy



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf