Hi,
On Friday, April 5, 2013, Takashi Iwai tiwai@suse.de wrote:
At Fri, 5 Apr 2013 19:02:08 +0530, mateen wrote:
Hi, I checked with Takshi's patch and would like to suggest couple of changes.
- We keep dmix_down_sem(dmix) and dmix_up_sem(dmix) as macros instead of
defining them as functions.
No, no. That's a very bad practice. Don't do it. If any, use static inline.
Agreed.
But in this case, there is no merit to do inline. Let compiler optimize.
- We free up the semaphore in snd_pcm_dmix_close() if it is recognized
that the semaphore is held up by the same process, which will be indicated by dmix->semlocked flag.
Why do you need reacquire the very same lock at all...? I see no point for it.
Agree, That lock is still with the same process.we need not reacquire it.
Takashi
Mateen.
diff -Nuar dir2/pcm_direct.h dir1/pcm_direct.h --- dir2/pcm_direct.h 2009-12-16 20:48:51.000000000 +0530 +++ dir1/pcm_direct.h 2013-04-05 17:06:48.331497000 +0530 @@ -123,6 +123,7 @@ int ipc_gid; /* IPC socket gid */ int semid; /* IPC global semaphore identification */ int shmid; /* IPC global shared memory identification */
- int semlocked;
snd_pcm_direct_share_t *shmptr; /* pointer to shared memory area */ snd_pcm_t *spcm; /* slave PCM handle */ snd_pcm_uframes_t appl_ptr; diff -Nuar dir2/pcm_dmix.c dir1/pcm_dmix.c --- dir2/pcm_dmix.c 2009-12-16 20:48:51.000000000 +0530 +++ dir1/pcm_dmix.c 2013-04-05 17:04:02.781109000 +0530 @@ -285,8 +285,17 @@ */ #ifndef DOC_HIDDEN #ifdef NO_CONCURRENT_ACCESS -#define dmix_down_sem(dmix) snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT) -#define dmix_up_sem(dmix) snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT) +#define dmix_down_sem(dmix) \ +{\ +if (!dmix->semlocked++)\
- snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);\
+} +#define dmix_up_sem(dmix) \ +{\
- if (!--dmix->semlocked)\
- snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);\
+}
#else #define dmix_down_sem(dmix) #define dmix_up_sem(dmix) @@ -764,7 +773,13 @@
if (dmix->timer) snd_timer_close(dmix->timer);
- snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
- if(!dmix->semlocked)
- snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
- else{
- snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
- snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
- }
snd_pcm_close(dmix->spcm); if (dmix->server) snd_pcm_direct_server_discard(dmix);
Message: 1 Date: Thu, 04 Apr 2013 18:30:01 +0200 From: Takashi Iwai tiwai@suse.de To: Jaroslav Kysela perex@perex.cz Cc: alsa-devel@alsa-project.org Subject: Re: [alsa-devel] Deadlock over semaphore issue with aplay while using dmix Message-ID: s5h38v645ti.wl%tiwai@suse.de Content-Type: text/plain; charset=US-ASCII
At Thu, 04 Apr 2013 13:33:08 +0200, Jaroslav Kysela wrote:
Date 4.4.2013 11:27, mateen wrote:
Hi,
I seeing sometimes deadlock issue with dmix when I press CTRL+C.
Aplay's signal handler calls snd_pcm_close() if an interrupt occurs. snd_pcm_close() will internally call pcm->ops->close() which will fall
to
snd_pcm_dmix_close() in case you are using dmix.
snd_pcm_dmix_close() will try to acquire the semaphore with snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT). The same semaphore is acquired in snd_pcm_dmix_sync_area() with dmix_down_sem() in case of non-concurrent access.
If semaphore is acquired in sn> [2 <text/html; ISO-8859-1 (quoted-printable)>]