On 2022-11-10 5:36 PM, Pierre-Louis Bossart wrote:
On 11/10/22 10:29, Cezary Rojewski wrote:
On 2022-11-10 5:18 PM, Pierre-Louis Bossart wrote:
On 11/10/22 10:06, Cezary Rojewski wrote:
Unless something new has been added/updated, there is no runtime->state field available. All the PCM code works with runtime->status->state.
cd sound/
git grep -c 'runtime->state' core/compress_offload.c:27
...
git grep -c 'status->state' core/pcm_compat.c:2 core/pcm_native.c:4
I see now, thanks. Commit from late September "ALSA: pcm: Avoid reference to status->state" add a new field. Will update the code to use __snd_pcm_set_state() instead.
My base did not have it yet.
Right, it's relatively recent, and my point is that the helper does use stream locking when testing the same state you modify. Maybe that's a red herring but I thought I'd ask.
static void snd_pcm_set_state(struct snd_pcm_substream *substream, snd_pcm_state_t state) { snd_pcm_stream_lock_irq(substream); if (substream->runtime->state != SNDRV_PCM_STATE_DISCONNECTED) __snd_pcm_set_state(substream->runtime, state); snd_pcm_stream_unlock_irq(substream); }
Your question is a right one and this is the right time to talk about locking. Our current test results and my analysis show that locking is not needed (what isn't the case for the first patch in the series) but races such as this one are hard to reproduce. If I'm proven wrong, no problem updating the code on my side.