Hi Clemens,

On Nov 17 2014 06:21, Clemens Ladisch wrote:
> Takashi Sakamoto wrote:
>> +++ b/sound/firewire/oxfw/oxfw-stream.c
>> +int snd_oxfw_stream_start_simplex(struct snd_oxfw *oxfw,
>> +				  struct amdtp_stream *stream,
>> +				  unsigned int rate, unsigned int pcm_channels)
>> +{
>> ...
>> +	if (atomic_read(substreams) == 0)
>> +		goto end;
>> +
>>  	mutex_lock(&oxfw->mutex);
> 
> What happens if hw_free is called between the calls to atomic_read() and
> mutex_lock()?

In the worst case, the codes after the mutex_lock() manage to start
duplex streams with released resources, then it causes kernel panic.

This is a bug. I should have move atmic_read() to the critical section...

> And why are the substreams counters atomic?
> Can't they just be normal variables accessed from inside the mutex?

Just for my convinience. I didn't want to write many
mutex_lock()/mutex_unlock() because wrong coding of them causes-dead
lock, then push them inner stream.c. This idea is good except for
reference couters, so I added atomic_t.

An attached patch achieves your idea over my patchset. Handling
reference counter is easy to understand (because it's arithmetric
operation) but I don't like much mutex_lock()/mutex_unlock().

Can I request you to explain about the advantages of your idea?


Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp