Hello. Takashi Iwai wrote: >>> Anyway, what I meant is that the part touching I/O ports could be a faster path >>> than softirq. And the hrtimer code can use a bit simpler locks. The hrtimer >>> callback needs just one thing about DMA buffer - the buffer is allocated and >>> the address doesn't change during the callback. So, we'd need the lock for the >>> hr callback and for the PCM prepare, hwparams and free callbacks, i.e. to >>> assure that the hrtimer callback is finished when these PCM callbacks may >>> change the DMA buffer. Thus this lock doesn't have to be PCM substream lock. >> I wonder if it would be better to introduce >> such a lock in an alsa core rather than in a >> driver. The same way as snd_pcm_stream_lock(). > I don't think it's good to introduce yet another lock in the PCM > core. More lock, more complex. Even now the lock handling is too > complex (and might be buggy in some cases) in ALSA PCM core. Then I might be missing something. Right now I touch runtime->dma_area[] in a callback. You say it have to be protected, but not with alsa lock, but with my own lock. But I do not touch it ever in any other place. Everything else happens in an alsa core. So how would I possibly protect it with some lock private to snd-pcsp? > I think, however, it'd be worth to consider snd_pcm_period_elapsed() > to be softirq in PCM core so that the driver doesn't need > initializations. This will also reduce the spin unlock/re-lock > procedure in IRQ handler around snd_pcm_perild_elapsed(). I guess it will help, but such a solution has a tendency of being classified as a "stupid hack", which you've already seen recently. :) There really must be other ways, that do not carry such a risk. >> Wouldn't it be better to just make >> snd_pcm_period_elapsed() to not take >> the stream lock? There is a requirement >> right now to drop the stream lock before >> calling this. I still beleive this is >> the root of all the headache. > No. The work done by snd_pcm_period_elapsed() touches critical > sections and is delicated enough to protect by a lock. I didn't mean it should run without a lock. But it shouldn't take it itself, allowing the caller to _not drop_ it before calling. Is this acceptable? > The root of all the headache is the order of the spinlock. It's just > one lock in the PCM; and you must take it somewhere to protect. Certainly, I take it. But the unfortunate policy is that I have to drop it before calling snd_pcm_period_elapsed(). Can we change that? Of course, that will mean more changes than just to not take the lock in snd_pcm_period_elapsed(). Most likely the snd_pcm_stream_lock() and all its users will have to be slightly adjusted, but I think its pretty much a trivial changes, which will lead to a much cleaner code. And I remember the last time we discussed this, you admitted (some of) the other drivers do get this part wrong too. So fixing it properly once and for all might be a good idea as you are at it again. _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
