ALSA Memory Allocation Bug

ALSA Memory Allocation Bug

[Loc Ho]

Hi,

I developed an custom USB Audio driver. In addition, I am also using the provided USB Audio driver for commercial USB Audio devices. After some testing with the custom USB Audio driver and the provided USB Audio driver, there seem to be a problem with the ALSA memory allocation routine. In file pcm_memory.c function setup_pcm_id, the id is generated using the pcm device ID, stream ID, and substream number. This only guarantee that the memory tag ID is unique for one card. If I have multiple cards, they will not be unique. Am I correct?

Thanks,
Loc


-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id%62&alloc_ida84&op=click

Re: ALSA Memory Allocation Bug

[Takashi Iwai]

At Wed, 12 May 2004 18:40:05 -0700,
Loc Ho wrote:
> 
> Hi,
> 
> I developed an custom USB Audio driver. In addition, I am also using
> the provided USB Audio driver for commercial USB Audio devices. After
> some testing with the custom USB Audio driver and the provided USB
> Audio driver, there seem to be a problem with the ALSA memory
> allocation routine. In file pcm_memory.c function setup_pcm_id, the id
> is generated using the pcm device ID, stream ID, and substream
> number. This only guarantee that the memory tag ID is unique for one
> card. If I have multiple cards, they will not be unique. Am I correct?

no, usually you have a unique struct device pointer for each card, so
it's no problem.  the tag is a pair of device pointer and id number.

however, if the same struct device is shared among multiple card
instances, you'll need to set up the id field by yourself.  an
exmample is found in mixart driver.


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click

Re: ALSA Memory Allocation Bug

[Takashi Iwai]

At Thu, 13 May 2004 10:45:20 -0700,
Loc Ho wrote:
> 
> Hi,
> 
> Are you saying that the third parameter for the function
> snd_pcm_new, int device, should be the value of the "chip" (driver
> context pointer) pointer.

no.

as written in my last mail, both dev and id fields of struct
snd_dma_device are used as the "tag" of a buffer.  since dev is a
struct device pointer and is usually unique between card instances,
they don't conflict even if id field is identical.


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by: SourceForge.net Broadband
Sign-up now for SourceForge Broadband and get the fastest
6.0/768 connection for only $19.95/mo for the first 3 months!
http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click

Re: ALSA Memory Allocation Bug

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 540 bytes --]

At Thu, 13 May 2004 10:53:48 -0700,
Loc Ho wrote:
> 
> Hi,
> 
> Also, after looking at the snd memory allocation routines, the
> unique tag are type, id, and flag. The type is "continous". The id is
> formated by the device ID, stream number, and substream number. The
> flag is the kernel memory flag. I already discussed the ID in my
> previous email (see below). Therefore, it will NOT be unique across
> multiple card!!!  

hmm, you're right.

obviously it's a bug of snd-usb-audio.
the attached patch should fix the problem.


Takashi

[-- Attachment #2: Type: text/plain, Size: 1185 bytes --]

Index: alsa-kernel/usb/usbaudio.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/usb/usbaudio.c,v
retrieving revision 1.100
diff -u -r1.100 usbaudio.c
--- alsa-kernel/usb/usbaudio.c	7 May 2004 10:08:49 -0000	1.100
+++ alsa-kernel/usb/usbaudio.c	13 May 2004 18:05:45 -0000
@@ -2036,6 +2036,7 @@
 static void init_substream(snd_usb_stream_t *as, int stream, struct audioformat *fp)
 {
 	snd_usb_substream_t *subs = &as->substream[stream];
+	snd_pcm_substream_t *pcms;
 
 	INIT_LIST_HEAD(&subs->fmt_list);
 	spin_lock_init(&subs->lock);
@@ -2047,7 +2048,12 @@
 		subs->ops = audio_urb_ops[stream];
 	else
 		subs->ops = audio_urb_ops_high_speed[stream];
-	snd_pcm_lib_preallocate_pages(as->pcm->streams[stream].substream,
+	/* set up the unique device id with the card index number */
+	pcms = as->pcm->streams[stream].substream;
+	pcms->dma_device.id = (as->pcm->device << 16) |
+		(pcms->stream << 8) | (pcms->number + 1) |
+		(as->chip->card->number + 1) << 24;
+	snd_pcm_lib_preallocate_pages(pcms,
 				      SNDRV_DMA_TYPE_CONTINUOUS,
 				      snd_dma_continuous_data(GFP_KERNEL),
 				      64 * 1024, 128 * 1024);

Re: ALSA Memory Allocation Bug

[Takashi Iwai]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

At Thu, 13 May 2004 20:06:45 +0200,
I wrote:
> 
> [1  <text/plain; US-ASCII (7bit)>]
> At Thu, 13 May 2004 10:53:48 -0700,
> Loc Ho wrote:
> > 
> > Hi,
> > 
> > Also, after looking at the snd memory allocation routines, the
> > unique tag are type, id, and flag. The type is "continous". The id is
> > formated by the device ID, stream number, and substream number. The
> > flag is the kernel memory flag. I already discussed the ID in my
> > previous email (see below). Therefore, it will NOT be unique across
> > multiple card!!!  
> 
> hmm, you're right.
> 
> obviously it's a bug of snd-usb-audio.
> the attached patch should fix the problem.

maybe this one is better.
i'll commit this to cvs.
	

Takashi

[-- Attachment #2: Type: text/plain, Size: 767 bytes --]

Index: alsa-kernel/core/pcm_memory.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/core/pcm_memory.c,v
retrieving revision 1.25
diff -u -r1.25 pcm_memory.c
--- alsa-kernel/core/pcm_memory.c	7 Apr 2004 17:49:39 -0000	1.25
+++ alsa-kernel/core/pcm_memory.c	13 May 2004 18:16:27 -0000
@@ -224,9 +224,13 @@
  */
 static inline void setup_pcm_id(snd_pcm_substream_t *subs)
 {
-	if (! subs->dma_device.id)
+	if (! subs->dma_device.id) {
 		subs->dma_device.id = subs->pcm->device << 16 |
 			subs->stream << 8 | (subs->number + 1);
+		if (subs->dma_device.type == SNDRV_DMA_TYPE_CONTINUOUS ||
+		    subs->dma_device.dev == NULL)
+			subs->dma_device.id |= (subs->pcm->card->number + 1) << 24;
+	}
 }
 
 /**