On Nov 15 2014 21:04, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
If I rewrite your pseudo code, like this:
void start_some_substream(...) { if (atomic_inc_and_test(substreams)) { mutex_lock(); // lots of stuff to start DMA ...
if (wait_for_first_packet() == ETIMEDOUT) { stop_dma(); release_streams(); mutex_unlock(); return ETIMEDOUT; }mutex_unlock(); } ...}
This is an even better example of why atomic_t is incorrect: when the startup fails, the second call to start_some_substream() at the same time, which just sees that substreams>0, cannot notice the error, and thinks its own substream has been started successfully.
Yes. The reference counter is invalid in this pseudo code which I showrf. I left your insistence about 'atomic_inc_and_test()' what it is.
More precisely, current implementations (bebob/fireworks/dice) are (or should be for oxfw) like:
void start_some_substream(...) { mutex_lock();
if (atomic_read(substreams) == 0) goto end;
if (streams_are_running()) goto end;
if (!start_isochronous_context_and_dma()) { release_isochronous_context_and_dma(); release_streams(); goto end; }
if (wait_for_first_packet() == ETIMEDOUT) { release_isochronous_context_and_dma(); release_streams(); } end: mutex_unlock(); return err; }
I believe that the reference counter with atomic_t is valid regardless of inner/outer the lock (mutex). For this critical section, it's just a importance that some substreams already subscribe the stream or not, instead of the number of substreams.
If the reference counter is non-zero, it means that 'a stream is requested to run for some substream, even if the substreams are not for current caller'.
I don't show in the pseudo code; when the streams are already running but the sampling transfer frequency is not the same as the one which the caller requested, then the streams are stopped once and set sampling transfer frequency and restart them.
Regards
Takashi Sakamoto o-takashi@sakamocchi.jp