On Tue, 17 May 2016 15:29:33 +0200,
Ricard Wanderlof wrote:
>
>
> On Tue, 17 May 2016, Takashi Iwai wrote:
>
> > There is a manual lock loop with snd_atomic_read_wait(), as found in
> > pcm_plugin.c. So, removing the stuff may potentially damage some apps
> > that wrongly assume the thread safety.
>
> Yes, I think could be true. Certainly GStreamer seems to assume this.
>
> > Now I've been considering this issue again, I'm inclined to suggest:
> > let's begin with deprecating the atomic stuff there, but opt-in via
> > via a configure option for a while, in case someone still really needs
> > it.
> >
> > By making it optional, it's fine to take your patch and cleanup the
> > old code. Then we don't have to think of the compatibility with the
> > old toolchain so much.
>
> I'm trying to consider how this would be done in practice. Basically, add
> a configuration option like '--with-atomic' with the codec in iatomic.h
> looking something like:
>
> static __inline__ void snd_atomic_write_begin(snd_atomic_write_t *w)
> {
> + #ifdef USE_ATOMIC
> + __atomic_add_fetch(&w->begin, 1,__ATOMIC_SEQ_CST);
> + #else
> - w->begin++;
> + #endif
> - wmb();
> }
>
> Thus removing the (useless?) wmb() in any case, and going to w->begin++ in
> the non-atomic case, and __atomic_add_fetch() in the atomic case?
No, my proposal is to make all snd_atomic_*() NOP unless a configure
option is passed.
> However, this does mean that those that need/expect atomic operations must
> explicitly add a new configuration option, but perhaps that is ok, as one
> could argue that the applications that need it are buggy to start with.
Right. We can spot out whether the application bugs, and yet some
excuse for breakage :)
> Also, it would prohibit building the building of alsa-lib for such
> applications with gcc < 4.7 (when __atomic_add_fetch was introduced), but
> perhaps that's more of an incentive than anything else. :-)
Yep.
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
