Hi all,
this are the sound locking fixes for es1371. While it is inspired by the
trident fixes it contains some changes over it:
o es1371_mmap used to use lock_kernel to do some synchronistation,
this is superceeded by s->sem.
o remap_page_range (used in es1371_mmap) needs the mm semaphore as
stated by a comment and the code. I have found _NO_ driver in the
tree so far that does this locking right...
Looks like we have another round of races...
Christoph
--
Of course it doesn't work. We've performed a software upgrade.
--- linux-2.4.5-ac6/drivers/sound/es1371.c Sat Jun 2 14:35:08 2001
+++ linux/drivers/sound/es1371.c Sat Jun 2 17:35:11 2001
@@ -417,8 +417,16 @@
unsigned sctrl;
unsigned dac1rate, dac2rate, adcrate;
+ /*
+ * Lock order (high->low):
+ * - lock hardware lock
+ * - open_sem guards opens
+ * - sem guard dmabuf, write re-entry etc
+ */
spinlock_t lock;
struct semaphore open_sem;
+ struct semaphore sem;
+
mode_t open_mode;
wait_queue_head_t open_wait;
@@ -1336,21 +1344,27 @@
{
struct es1371_state *s = (struct es1371_state *)file->private_data;
DECLARE_WAITQUEUE(wait, current);
- ssize_t ret;
+ ssize_t ret = 0;
unsigned long flags;
unsigned swptr;
int cnt;
VALIDATE_STATE(s);
+
if (ppos != &file->f_pos)
return -ESPIPE;
if (s->dma_adc.mapped)
return -ENXIO;
- if (!s->dma_adc.ready && (ret = prog_dmabuf_adc(s)))
- return ret;
if (!access_ok(VERIFY_WRITE, buffer, count))
return -EFAULT;
- ret = 0;
+
+ /*
+ * Guard against concurrent dmabuf programming.
+ */
+ down(&s->sem);
+ if (!s->dma_adc.ready && (ret = prog_dmabuf_adc(s)))
+ goto up_and_out;
+
add_wait_queue(&s->dma_adc.wait, &wait);
while (count > 0) {
spin_lock_irqsave(&s->lock, flags);
@@ -1371,12 +1385,14 @@
ret = -EAGAIN;
break;
}
+ up(&s->sem);
schedule();
if (signal_pending(current)) {
if (!ret)
ret = -ERESTARTSYS;
- break;
+ goto out;
}
+ down(&s->sem);
continue;
}
if (copy_to_user(buffer, s->dma_adc.rawbuf + swptr, cnt)) {
@@ -1395,29 +1411,45 @@
if (s->dma_adc.enabled)
start_adc(s);
}
+ up(&s->sem);
+
+out:
remove_wait_queue(&s->dma_adc.wait, &wait);
set_current_state(TASK_RUNNING);
return ret;
+up_and_out:
+ up(&s->sem);
+ return ret;
}
static ssize_t es1371_write(struct file *file, const char *buffer, size_t count,
loff_t *ppos)
{
struct es1371_state *s = (struct es1371_state *)file->private_data;
DECLARE_WAITQUEUE(wait, current);
- ssize_t ret;
+ ssize_t ret = -ENXIO;
unsigned long flags;
unsigned swptr;
int cnt;
VALIDATE_STATE(s);
+
if (ppos != &file->f_pos)
return -ESPIPE;
+
+ /*
+ * Guard against an mmap or ioctl while writing
+ */
+ down(&s->sem);
+
if (s->dma_dac2.mapped)
- return -ENXIO;
+ goto up_and_out;
if (!s->dma_dac2.ready && (ret = prog_dmabuf_dac2(s)))
- return ret;
- if (!access_ok(VERIFY_READ, buffer, count))
- return -EFAULT;
+ goto up_and_out;
+ if (!access_ok(VERIFY_READ, buffer, count)) {
+ ret = -EFAULT;
+ goto up_and_out;
+ }
+
ret = 0;
add_wait_queue(&s->dma_dac2.wait, &wait);
while (count > 0) {
@@ -1443,12 +1475,14 @@
ret = -EAGAIN;
break;
}
+ up(&s->sem);
schedule();
if (signal_pending(current)) {
if (!ret)
ret = -ERESTARTSYS;
- break;
+ goto out;
}
+ down(&s->sem);
continue;
}
if (copy_from_user(s->dma_dac2.rawbuf + swptr, buffer, cnt)) {
@@ -1468,9 +1502,15 @@
if (s->dma_dac2.enabled)
start_dac2(s);
}
+ up(&s->sem);
+
+out:
remove_wait_queue(&s->dma_dac2.wait, &wait);
set_current_state(TASK_RUNNING);
return ret;
+up_and_out:
+ up(&s->sem);
+ return ret;
}
/* No kernel lock - we have our own spinlock */
@@ -1481,21 +1521,29 @@
unsigned int mask = 0;
VALIDATE_STATE(s);
+
+ /*
+ * Guard against a parallel poll and write causing multiple
+ * prog_dmabuf events
+ */
+ down(&s->sem);
if (file->f_mode & FMODE_WRITE) {
if (!s->dma_dac2.ready && prog_dmabuf_dac2(s))
- return 0;
+ goto up_and_out;
poll_wait(file, &s->dma_dac2.wait, wait);
}
if (file->f_mode & FMODE_READ) {
if (!s->dma_adc.ready && prog_dmabuf_adc(s))
- return 0;
+ goto up_and_out;
poll_wait(file, &s->dma_adc.wait, wait);
}
+ up(&s->sem);
+
spin_lock_irqsave(&s->lock, flags);
es1371_update_ptr(s);
if (file->f_mode & FMODE_READ) {
- if (s->dma_adc.count >= (signed)s->dma_adc.fragsize)
- mask |= POLLIN | POLLRDNORM;
+ if (s->dma_adc.count >= (signed)s->dma_adc.fragsize)
+ mask |= POLLIN | POLLRDNORM;
}
if (file->f_mode & FMODE_WRITE) {
if (s->dma_dac2.mapped) {
@@ -1506,51 +1554,58 @@
mask |= POLLOUT | POLLWRNORM;
}
}
+
spin_unlock_irqrestore(&s->lock, flags);
return mask;
+up_and_out:
+ up(&s->sem);
+ return 0;
}
static int es1371_mmap(struct file *file, struct vm_area_struct *vma)
{
struct es1371_state *s = (struct es1371_state *)file->private_data;
- struct dmabuf *db;
+ struct dmabuf *db = NULL;
int ret;
unsigned long size;
VALIDATE_STATE(s);
- lock_kernel();
+
+ /*
+ * Lock against poll read write or mmap creating buffers. Also lock
+ * a read or write against an mmap.
+ */
+ down(&s->sem);
if (vma->vm_flags & VM_WRITE) {
- if ((ret = prog_dmabuf_dac2(s)) != 0) {
- unlock_kernel();
- return ret;
- }
+ if ((ret = prog_dmabuf_dac2(s)) != 0)
+ goto out;
db = &s->dma_dac2;
} else if (vma->vm_flags & VM_READ) {
- if ((ret = prog_dmabuf_adc(s)) != 0) {
- unlock_kernel();
- return ret;
- }
+ if ((ret = prog_dmabuf_adc(s)) != 0)
+ goto out;
db = &s->dma_adc;
- } else {
- unlock_kernel();
- return -EINVAL;
- }
- if (vma->vm_pgoff != 0) {
- unlock_kernel();
- return -EINVAL;
}
+
+ ret = -EINVAL;
+ if (db == NULL)
+ goto out;
+ if (vma->vm_pgoff != 0)
+ goto out;
+
size = vma->vm_end - vma->vm_start;
- if (size > (PAGE_SIZE << db->buforder)) {
- unlock_kernel();
- return -EINVAL;
- }
- if (remap_page_range(vma->vm_start, virt_to_phys(db->rawbuf), size,
vma->vm_page_prot)) {
- unlock_kernel();
- return -EAGAIN;
- }
- db->mapped = 1;
- unlock_kernel();
- return 0;
+ if (size > (PAGE_SIZE << db->buforder))
+ goto out;
+
+ down_write(¤t->mm->mmap_sem);
+ ret = remap_page_range(vma->vm_start, virt_to_phys(db->rawbuf),
+ size, vma->vm_page_prot);
+ up_write(¤t->mm->mmap_sem);
+
+ if (ret == 0)
+ db->mapped = 1;
+out:
+ up(&s->sem);
+ return ret;
}
static int es1371_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
unsigned long arg)
@@ -2787,8 +2842,9 @@
init_waitqueue_head(&s->open_wait);
init_waitqueue_head(&s->midi.iwait);
init_waitqueue_head(&s->midi.owait);
- init_MUTEX(&s->open_sem);
spin_lock_init(&s->lock);
+ init_MUTEX(&s->open_sem);
+ init_MUTEX(&s->sem);
s->magic = ES1371_MAGIC;
s->dev = pcidev;
s->io = pci_resource_start(pcidev, 0);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
