Re: [PATCH - JACK 1/1] jack: Use internal snd_pcm_state to reduce amount of additional variables

From: Takashi Iwai <tiwai@suse.de>
To: "Wischer, Timo (ADITG/ESB)" <twischer@de.adit-jv.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Subject: Re: [PATCH - JACK 1/1] jack: Use internal snd_pcm_state to reduce amount of additional variables
Date: Thu, 15 Mar 2018 16:49:54 +0100	[thread overview]
Message-ID: <s5h605x72zh.wl-tiwai@suse.de> (raw)
In-Reply-To: <B0FB33DC1499054591F62C0EF1E013D7684FDCEB@HI2EXCH01.adit-jv.com>

On Thu, 15 Mar 2018 16:07:31 +0100,
Wischer, Timo (ADITG/ESB) wrote:
> 
> > We need to check it, of course.  But the behavior of the prepare
> > function is certainly different from others, so it's more likely a
> > bug.
> 
> I could provide a fix for the IO plug API for prepare and draining.

Yes, these are very trivial changes.

> > This should be notified to ioplug via snd_pcm_ioplug_set_state().
> 
> Without an additional variable snd_pcm_ioplug_set_state() has to be called from the JACK thread.
> But snd_pcm_ioplug_set_state() is not thread safe.

Well, but your internal state copy is also not thread safe as of this
patch, after all.

> > That'd be great if everything is ready, but this can't be a reason to
> > add a shadow state and modify the code as in the patch for now.
> 
> At the end I would like to use functions similar to READ_ONCE() and WRITE_ONCE() to access the state (known from the Linux kernel [1]).
> But I think this should not be part of the ALSA library because it is for most IO plugins not required.
> Therefore I need to have an internal copy of the state which will only be accessed with READ_ONCE() and WRITE_ONCE().
> 
> You are right, it is not yet required but at the end I have to implement it.
> 
> Do you think we should implement such a synchronization into the ALSA lib?
> 
> [1] https://www.kernel.org/doc/Documentation/memory-barriers.txt

I really would like to avoid that, at least, not introducing any
dependency into alsa-lib.

The usage inside a plugin is another question, and it would depend on
the implementation details.  A dependency in plugin may cause a messy
conflict when different versions are used in a same application.  So,
in general, it'd be best for a plugin to be self-contained.

Takashi