At Wed, 7 Aug 2013 13:38:20 -0400 (EDT), Alan Stern wrote:

On Wed, 7 Aug 2013, Takashi Iwai wrote:

...

[Cc'ed to linux-usb ML]

At Wed, 7 Aug 2013 16:51:49 +0200, Torsten Schenk wrote:

...

Patch fixes URB transfer buffer allocation for midi output to be DMA-able.

Is this really needed? That is, can't a transfer buffer be at middle of kmalloc'ed space, but must be always the head of the kmalloc'ed space?

A buffer _can_ be in the middle of a kmalloc'ed space, but the CPU must not access any of the fields around it that might occupy the same cache line while the buffer is being used for DMA. In general, it's safest not to put any other data in the same kmalloc'ed region with a DMA buffer.

Hrm, but does the kmalloc buffer always guarantee such cache line exclusiveness...? I thought a simple one like SLOB doesn't care.

...

...

@@ -32,7 +28,7 @@ struct snd_rawmidi_substream *out; struct urb out_urb; u8 out_serial; /* serial number of out packet */

• u8 out_buffer[MIDI_BUFSIZE];

• u8 *out_buffer; int buffer_offset;

In this case, the CPU would access out_urb while out_buffer was in use.

OK, then we need to fix sound/usb/6fire/pcm.c, too. Torsten, care to respin the patch?

thanks,

Takashi

At Sun, 11 Aug 2013 11:11:19 +0200, Torsten Schenk wrote:

Patch makes pcm buffers DMA-able by allocating each one separately.

Signed-off-by: Torsten Schenk torsten.schenk@zoho.com

Thanks, applied.

Takashi

---

diff -Nur a/sound/usb/6fire/pcm.c b/sound/usb/6fire/pcm.c --- a/sound/usb/6fire/pcm.c 2013-08-04 22:46:46.000000000 +0200 +++ b/sound/usb/6fire/pcm.c 2013-08-11 11:02:38.305774934 +0200 @@ -582,6 +582,33 @@ urb->instance.number_of_packets = PCM_N_PACKETS_PER_URB; }

+static int usb6fire_pcm_buffers_init(struct pcm_runtime *rt) +{

• int i;
• for (i = 0; i < PCM_N_URBS; i++) {

• rt->out_urbs[i].buffer = kzalloc(PCM_N_PACKETS_PER_URB
  

• 		* PCM_MAX_PACKET_SIZE, GFP_KERNEL);
  

• if (!rt->out_urbs[i].buffer)
  

• 	return -ENOMEM;
  

• rt->in_urbs[i].buffer = kzalloc(PCM_N_PACKETS_PER_URB
  

• 		* PCM_MAX_PACKET_SIZE, GFP_KERNEL);
  

• if (!rt->in_urbs[i].buffer)
  

• 	return -ENOMEM;
  

• }
• return 0;

+}

+static void usb6fire_pcm_buffers_destroy(struct pcm_runtime *rt) +{

• int i;
• for (i = 0; i < PCM_N_URBS; i++) {

• kfree(rt->out_urbs[i].buffer);
  

• kfree(rt->in_urbs[i].buffer);
  

• }

+}

int usb6fire_pcm_init(struct sfire_chip *chip) { int i; @@ -593,6 +620,13 @@ if (!rt) return -ENOMEM;

• ret = usb6fire_pcm_buffers_init(rt);
• if (ret) {

• usb6fire_pcm_buffers_destroy(rt);
  

• kfree(rt);
  

• return ret;
  

• }
• rt->chip = chip; rt->stream_state = STREAM_DISABLED; rt->rate = ARRAY_SIZE(rates);

@@ -614,6 +648,7 @@

ret = snd_pcm_new(chip->card, "DMX6FireUSB", 0, 1, 1, &pcm); if (ret < 0) {

• usb6fire_pcm_buffers_destroy(rt);
  

  kfree(rt); snd_printk(KERN_ERR PREFIX "cannot create pcm instance.\n"); return ret;

@@ -625,6 +660,7 @@ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_ops);

if (ret) {

• usb6fire_pcm_buffers_destroy(rt);
  

  kfree(rt); snd_printk(KERN_ERR PREFIX "error preallocating pcm buffers.\n");

@@ -669,6 +705,9 @@

void usb6fire_pcm_destroy(struct sfire_chip *chip) {

• kfree(chip->pcm);

• struct pcm_runtime *rt = chip->pcm;
• usb6fire_pcm_buffers_destroy(rt);
• kfree(rt); chip->pcm = NULL;

} diff -Nur a/sound/usb/6fire/pcm.h b/sound/usb/6fire/pcm.h --- a/sound/usb/6fire/pcm.h 2013-08-04 22:46:46.000000000 +0200 +++ b/sound/usb/6fire/pcm.h 2013-08-11 10:49:28.779700620 +0200 @@ -32,7 +32,7 @@ struct urb instance; struct usb_iso_packet_descriptor packets[PCM_N_PACKETS_PER_URB]; /* END DO NOT SEPARATE */

• u8 buffer[PCM_N_PACKETS_PER_URB * PCM_MAX_PACKET_SIZE];

• u8 *buffer;

  struct pcm_urb *peer;

};