[alsa-devel] snd_pcsp locking mess
tiwai at suse.de
Mon Jun 2 11:36:34 CEST 2008
At Thu, 29 May 2008 21:07:55 +0400,
Stas Sergeev wrote:
> Takashi Iwai wrote:
> >> really be better if just snd_pcm_period_elapsed()
> >> would raise a softirq. That's something
> >> you proposed in the past IIRC.
> >> Is this acceptable to have that change
> >> not in the driver, or do you think it
> >> is snd-pcsp-specific, and no other driver
> >> will benefit?
> > It would be good in general.
> > However, better to be the next step after changing pcsp.
> But why do you need both changes?
> If you are going to change snd_pcm_period_elapsed()
> itself, then how would the snd-pcsp
> change be justified?
snd-pcsp needs anyway the change to HRTIMER_IRQSAFE again.
And, moving snd_pcm_period_elapsed() to a tasklet isn't needed for
RT kernels. It's basically a good thing, but just not mandatory like
> > One big disadvantage of the softirq is its (possible) timing
> > inaccuracy. (Note that this is about the normal non-preempt kernel.)
> > The thing like pcsp driver does, flipping the bits at a high rate,
> > isn't for softirq per se.
> OK, I see.
> > Well, it can be done in another away if you are so nervous about it.
> > Simply add chip->timer_running atomic_t and reset it when the callback
> > finished with HRTIMER_NORESTART. In another side, wait until
> > chip->timer_running becomes zero.
> > But, it's just an uneeded addition of the code.
> But for some reasons it looks safer
> to me...
A spinlock is like a gun: you can protect yourself with it, and you
can shoot others with it :)
> >>> Because the substream lock isn't necessary! It just makes things more
> >>> complicated -- if we take substream lock, AB/BA deadlocks occur again.
> >> Even if the callback is called within
> >> the softirq?
> > The runtime callbacks, pointer and trigger, are called already inside they
> > substream lock held by the PCM core. Thus it doesn't matter whether
> > it's in a hard or a softirq.
> The difference is that in a softirq,
> the hrtimer code doesn't take a lock.
But it may cause automatically latencies for beep controls.
The latency isn't a big issue for snd_pcm_period_elapsed() (unless you
want realtime thingy with snd-pcsp), but for beep controls, it does.
> The AB/BA problem was because of the
> hrtimer' lock, which is not there
> with the softirq callback. So what
> exactly A and B do you mean here?
That's what I meant for, too. Note that in the above text, I presume
that the hrtimer is called in HRTIMER_IRQSAFE.
> >> and yes, its nasty. But to me its nastiness
> >> is only because it needs another lock to
> >> protect the substream itself.
> > No, you don't need to protect the substream at all.
> But that was suggested by you in the
> past. :)
Yeah, that's my failure, obviously. I just thought your hrtimer
callback must be constantly called and hard to sync.
> But adding a syncronization
> point should be better I agree.
> I'll see if some minimal change is
> possible that will look sane to you
> and wont make me ask 100 questions
> about its safety at the same time. :)
More information about the Alsa-devel