[PATCH 0/3] further BKL removal

[PATCH 0/3] further BKL removal

[Arnd Bergmann]

With the ioctl, block, tty, vfs and llseek series
on their way into linux-next, these three patches
are attacking the hardest remaining issues.

If we get these ready for 2.6.36, the kernel should
be almost usable with the BKL disabled. In all three
cases, I'm cheating a bit, because the BKL still
remains lurking in the dark corners of the three
subsystems (i810/i830 for drm, OSS for sound and
lockd for fs/locks.c).

	Arnd

Cc: Christoph Hellwig <hch@lst.de>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: "J. Bruce Fields" <bfields@fieldses.org>
Cc: John Kacur <jkacur@redhat.com>
Cc: Matthew Wilcox <willy@linux.intel.com>
Cc: Miklos Szeredi <mszeredi@suse.cz>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: alsa-devel@alsa-project.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Arnd Bergmann (2):
  drm: kill BKL from common code
  sound: push BKL into open functions

Matthew Wilcox (1):
  Remove BKL from fs/locks.c

 arch/um/drivers/hostaudio_kern.c   |    6 ++
 drivers/gpu/drm/drm_drv.c          |    4 +-
 drivers/gpu/drm/drm_fops.c         |   23 ++++----
 drivers/gpu/drm/i810/i810_dma.c    |   44 +++++++++-----
 drivers/gpu/drm/i810/i810_drv.c    |    2 +-
 drivers/gpu/drm/i810/i810_drv.h    |    1 +
 drivers/gpu/drm/i830/i830_dma.c    |   42 +++++++++----
 drivers/gpu/drm/i830/i830_drv.c    |    2 +-
 drivers/gpu/drm/i830/i830_drv.h    |    1 +
 fs/Kconfig                         |    4 +
 fs/locks.c                         |  116 ++++++++++++++++++++++--------------
 include/drm/drmP.h                 |    2 +-
 sound/core/hwdep.c                 |   14 +++-
 sound/core/oss/mixer_oss.c         |   19 ++++---
 sound/oss/au1550_ac97.c            |   26 +++++---
 sound/oss/dmasound/dmasound_core.c |   28 +++++++--
 sound/oss/msnd_pinnacle.c          |   10 ++-
 sound/oss/sh_dac_audio.c           |    9 ++-
 sound/oss/soundcard.c              |   20 ++++---
 sound/oss/swarm_cs4297a.c          |   17 +++++-
 sound/oss/vwsnd.c                  |    8 +++
 sound/sound_core.c                 |    6 +--
 22 files changed, 267 insertions(+), 137 deletions(-)

[PATCH 3/3] sound: push BKL into open functions

[Arnd Bergmann]

This moves the lock_kernel() call from soundcore_open
to the individual OSS device drivers, where we can deal
with it one driver at a time if needed, or just kill
off the drivers.

All core components in ALSA already provide
adequate locking in their open()-functions
and do not require the big kernel lock, so
there is no need to add the BKL there.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: alsa-devel@alsa-project.org
---
 arch/um/drivers/hostaudio_kern.c   |    6 ++++++
 sound/core/hwdep.c                 |   14 ++++++++++----
 sound/core/oss/mixer_oss.c         |   19 +++++++++++--------
 sound/oss/au1550_ac97.c            |   26 +++++++++++++++++---------
 sound/oss/dmasound/dmasound_core.c |   28 ++++++++++++++++++++++------
 sound/oss/msnd_pinnacle.c          |   10 +++++++---
 sound/oss/sh_dac_audio.c           |    9 +++++++--
 sound/oss/soundcard.c              |   20 +++++++++++---------
 sound/oss/swarm_cs4297a.c          |   17 ++++++++++++++++-
 sound/oss/vwsnd.c                  |    8 ++++++++
 sound/sound_core.c                 |    6 +-----
 11 files changed, 116 insertions(+), 47 deletions(-)

diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c
index ae42695..68142df 100644
--- a/arch/um/drivers/hostaudio_kern.c
+++ b/arch/um/drivers/hostaudio_kern.c
@@ -8,6 +8,7 @@
 #include "linux/slab.h"
 #include "linux/sound.h"
 #include "linux/soundcard.h"
+#include "linux/smp_lock.h"
 #include "asm/uaccess.h"
 #include "init.h"
 #include "os.h"
@@ -198,7 +199,10 @@ static int hostaudio_open(struct inode *inode, struct file *file)
 	if (file->f_mode & FMODE_WRITE)
 		w = 1;
 
+	lock_kernel();
 	ret = os_open_file(dsp, of_set_rw(OPENFLAGS(), r, w), 0);
+	unlock_kernel();
+
 	if (ret < 0) {
 		kfree(state);
 		return ret;
@@ -254,7 +258,9 @@ static int hostmixer_open_mixdev(struct inode *inode, struct file *file)
 	if (file->f_mode & FMODE_WRITE)
 		w = 1;
 
+	lock_kernel();
 	ret = os_open_file(mixer, of_set_rw(OPENFLAGS(), r, w), 0);
+	unlock_kernel();
 
 	if (ret < 0) {
 		printk(KERN_ERR "hostaudio_open_mixdev failed to open '%s', "
diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c
index a70ee7f..3439587 100644
--- a/sound/core/hwdep.c
+++ b/sound/core/hwdep.c
@@ -94,13 +94,18 @@ static int snd_hwdep_open(struct inode *inode, struct file * file)
 		hw = snd_lookup_oss_minor_data(iminor(inode),
 					       SNDRV_OSS_DEVICE_TYPE_DMFM);
 #endif
-	} else
-		return -ENXIO;
+	} else {
+		err = -ENXIO;
+		goto out;
+	}
+
+	err = -ENODEV;
 	if (hw == NULL)
-		return -ENODEV;
+		goto out;
 
+	err = -EFAULT;
 	if (!try_module_get(hw->card->module))
-		return -EFAULT;
+		goto out;
 
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(&hw->open_wait, &wait);
@@ -147,6 +152,7 @@ static int snd_hwdep_open(struct inode *inode, struct file * file)
 	mutex_unlock(&hw->open_mutex);
 	if (err < 0)
 		module_put(hw->card->module);
+out:
 	return err;
 }
 
diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index f50ebf2..c30b1f3 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -49,17 +49,19 @@ static int snd_mixer_oss_open(struct inode *inode, struct file *file)
 
 	card = snd_lookup_oss_minor_data(iminor(inode),
 					 SNDRV_OSS_DEVICE_TYPE_MIXER);
-	if (card == NULL)
-		return -ENODEV;
-	if (card->mixer_oss == NULL)
-		return -ENODEV;
+	err = -ENODEV;
+	if (card == NULL || card->mixer_oss == NULL)
+		goto out;
+
 	err = snd_card_file_add(card, file);
 	if (err < 0)
-		return err;
+		goto out;
+
 	fmixer = kzalloc(sizeof(*fmixer), GFP_KERNEL);
 	if (fmixer == NULL) {
 		snd_card_file_remove(card, file);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto out;
 	}
 	fmixer->card = card;
 	fmixer->mixer = card->mixer_oss;
@@ -67,9 +69,10 @@ static int snd_mixer_oss_open(struct inode *inode, struct file *file)
 	if (!try_module_get(card->module)) {
 		kfree(fmixer);
 		snd_card_file_remove(card, file);
-		return -EFAULT;
+		err = -EFAULT;
 	}
-	return 0;
+out:
+	return err;
 }
 
 static int snd_mixer_oss_release(struct inode *inode, struct file *file)
diff --git a/sound/oss/au1550_ac97.c b/sound/oss/au1550_ac97.c
index c1070e3..47143a3 100644
--- a/sound/oss/au1550_ac97.c
+++ b/sound/oss/au1550_ac97.c
@@ -43,6 +43,7 @@
 #include <linux/sound.h>
 #include <linux/slab.h>
 #include <linux/soundcard.h>
+#include <linux/smp_lock.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -807,7 +808,9 @@ au1550_llseek(struct file *file, loff_t offset, int origin)
 static int
 au1550_open_mixdev(struct inode *inode, struct file *file)
 {
+	lock_kernel();
 	file->private_data = &au1550_state;
+	unlock_kernel();
 	return 0;
 }
 
@@ -1797,21 +1800,22 @@ au1550_open(struct inode *inode, struct file *file)
 #endif
 
 	file->private_data = s;
+	lock_kernel();
 	/* wait for device to become free */
 	mutex_lock(&s->open_mutex);
 	while (s->open_mode & file->f_mode) {
-		if (file->f_flags & O_NONBLOCK) {
-			mutex_unlock(&s->open_mutex);
-			return -EBUSY;
-		}
+		ret = -EBUSY;
+		if (file->f_flags & O_NONBLOCK)
+			goto out;		
 		add_wait_queue(&s->open_wait, &wait);
 		__set_current_state(TASK_INTERRUPTIBLE);
 		mutex_unlock(&s->open_mutex);
 		schedule();
 		remove_wait_queue(&s->open_wait, &wait);
 		set_current_state(TASK_RUNNING);
+		ret = -ERESTARTSYS;
 		if (signal_pending(current))
-			return -ERESTARTSYS;
+			goto out2;
 		mutex_lock(&s->open_mutex);
 	}
 
@@ -1840,17 +1844,21 @@ au1550_open(struct inode *inode, struct file *file)
 
 	if (file->f_mode & FMODE_READ) {
 		if ((ret = prog_dmabuf_adc(s)))
-			return ret;
+			goto out;
 	}
 	if (file->f_mode & FMODE_WRITE) {
 		if ((ret = prog_dmabuf_dac(s)))
-			return ret;
+			goto out;
 	}
 
 	s->open_mode |= file->f_mode & (FMODE_READ | FMODE_WRITE);
-	mutex_unlock(&s->open_mutex);
 	mutex_init(&s->sem);
-	return 0;
+	ret = 0;
+out:
+	mutex_unlock(&s->open_mutex);
+out2:
+	unlock_kernel();
+	return ret;
 }
 
 static int
diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index 3f3c3f7..5a4f38c 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -323,9 +323,13 @@ static struct {
 
 static int mixer_open(struct inode *inode, struct file *file)
 {
-	if (!try_module_get(dmasound.mach.owner))
+	lock_kernel();
+	if (!try_module_get(dmasound.mach.owner)) {
+		unlock_kernel();
 		return -ENODEV;
+	}
 	mixer.busy = 1;
+	unlock_kernel();
 	return 0;
 }
 
@@ -737,8 +741,11 @@ static int sq_open(struct inode *inode, struct file *file)
 {
 	int rc;
 
-	if (!try_module_get(dmasound.mach.owner))
+	lock_kernel();
+	if (!try_module_get(dmasound.mach.owner)) {
+		unlock_kernel();
 		return -ENODEV;
+	}
 
 	rc = write_sq_open(file); /* checks the f_mode */
 	if (rc)
@@ -781,10 +788,11 @@ static int sq_open(struct inode *inode, struct file *file)
 		sound_set_format(AFMT_MU_LAW);
 	}
 #endif
-
+	unlock_kernel();
 	return 0;
  out:
 	module_put(dmasound.mach.owner);
+	unlock_kernel();
 	return rc;
 }
 
@@ -1226,12 +1234,17 @@ static int state_open(struct inode *inode, struct file *file)
 {
 	char *buffer = state.buf;
 	int len = 0;
+	int ret;
 
+	lock_kernel();
+	ret = -EBUSY;
 	if (state.busy)
-		return -EBUSY;
+		goto out;
 
+	ret = -ENODEV;
 	if (!try_module_get(dmasound.mach.owner))
-		return -ENODEV;
+		goto out;
+
 	state.ptr = 0;
 	state.busy = 1;
 
@@ -1293,7 +1306,10 @@ printk("dmasound: stat buffer used %d bytes\n", len) ;
 		printk(KERN_ERR "dmasound_core: stat buffer overflowed!\n");
 
 	state.len = len;
-	return 0;
+	ret = 0;
+out:
+	unlock_kernel();
+	return ret;
 }
 
 static int state_release(struct inode *inode, struct file *file)
diff --git a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c
index a1e3f96..153d822 100644
--- a/sound/oss/msnd_pinnacle.c
+++ b/sound/oss/msnd_pinnacle.c
@@ -756,12 +756,15 @@ static int dev_open(struct inode *inode, struct file *file)
 	int minor = iminor(inode);
 	int err = 0;
 
+	lock_kernel();
 	if (minor == dev.dsp_minor) {
 		if ((file->f_mode & FMODE_WRITE &&
 		     test_bit(F_AUDIO_WRITE_INUSE, &dev.flags)) ||
 		    (file->f_mode & FMODE_READ &&
-		     test_bit(F_AUDIO_READ_INUSE, &dev.flags)))
-			return -EBUSY;
+		     test_bit(F_AUDIO_READ_INUSE, &dev.flags))) {
+			err = -EBUSY;
+			goto out;
+		}
 
 		if ((err = dsp_open(file)) >= 0) {
 			dev.nresets = 0;
@@ -782,7 +785,8 @@ static int dev_open(struct inode *inode, struct file *file)
 		/* nothing */
 	} else
 		err = -EINVAL;
-
+out:
+	unlock_kernel();
 	return err;
 }
 
diff --git a/sound/oss/sh_dac_audio.c b/sound/oss/sh_dac_audio.c
index 4153752..8f0be40 100644
--- a/sound/oss/sh_dac_audio.c
+++ b/sound/oss/sh_dac_audio.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/fs.h>
 #include <linux/sound.h>
+#include <linux/smp_lock.h>
 #include <linux/soundcard.h>
 #include <linux/interrupt.h>
 #include <linux/hrtimer.h>
@@ -216,13 +217,17 @@ static int dac_audio_open(struct inode *inode, struct file *file)
 {
 	if (file->f_mode & FMODE_READ)
 		return -ENODEV;
-	if (in_use)
+
+	lock_kernel();
+	if (in_use) {
+		unlock_kernel();
 		return -EBUSY;
+	}
 
 	in_use = 1;
 
 	dac_audio_start();
-
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/oss/soundcard.c b/sound/oss/soundcard.c
index 2d9c513..92aa762 100644
--- a/sound/oss/soundcard.c
+++ b/sound/oss/soundcard.c
@@ -210,42 +210,44 @@ static int sound_open(struct inode *inode, struct file *file)
 		printk(KERN_ERR "Invalid minor device %d\n", dev);
 		return -ENXIO;
 	}
+	lock_kernel();
 	switch (dev & 0x0f) {
 	case SND_DEV_CTL:
 		dev >>= 4;
 		if (dev >= 0 && dev < MAX_MIXER_DEV && mixer_devs[dev] == NULL) {
 			request_module("mixer%d", dev);
 		}
+		retval = -ENXIO;
 		if (dev && (dev >= num_mixers || mixer_devs[dev] == NULL))
-			return -ENXIO;
+			break;
 	
 		if (!try_module_get(mixer_devs[dev]->owner))
-			return -ENXIO;
+			break;
+
+		retval = 0;
 		break;
 
 	case SND_DEV_SEQ:
 	case SND_DEV_SEQ2:
-		if ((retval = sequencer_open(dev, file)) < 0)
-			return retval;
+		retval = sequencer_open(dev, file);
 		break;
 
 	case SND_DEV_MIDIN:
-		if ((retval = MIDIbuf_open(dev, file)) < 0)
-			return retval;
+		retval = MIDIbuf_open(dev, file);
 		break;
 
 	case SND_DEV_DSP:
 	case SND_DEV_DSP16:
 	case SND_DEV_AUDIO:
-		if ((retval = audio_open(dev, file)) < 0)
-			return retval;
+		retval = audio_open(dev, file);
 		break;
 
 	default:
 		printk(KERN_ERR "Invalid minor device %d\n", dev);
-		return -ENXIO;
+		retval = -ENXIO;
 	}
 
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/oss/swarm_cs4297a.c b/sound/oss/swarm_cs4297a.c
index 3136c88..34b0838 100644
--- a/sound/oss/swarm_cs4297a.c
+++ b/sound/oss/swarm_cs4297a.c
@@ -68,6 +68,7 @@
 #include <linux/delay.h>
 #include <linux/sound.h>
 #include <linux/slab.h>
+#include <linux/smp_lock.h>
 #include <linux/soundcard.h>
 #include <linux/ac97_codec.h>
 #include <linux/pci.h>
@@ -1534,6 +1535,7 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 	CS_DBGOUT(CS_FUNCTION | CS_OPEN, 4,
 		  printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()+\n"));
 
+	lock_kernel();
 	list_for_each(entry, &cs4297a_devs)
 	{
 		s = list_entry(entry, struct cs4297a_state, list);
@@ -1544,6 +1546,8 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 	{
 		CS_DBGOUT(CS_FUNCTION | CS_OPEN | CS_ERROR, 2,
 			printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()- -ENODEV\n"));
+
+		unlock_kernel();
 		return -ENODEV;
 	}
 	VALIDATE_STATE(s);
@@ -1551,6 +1555,7 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 
 	CS_DBGOUT(CS_FUNCTION | CS_OPEN, 4,
 		  printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()- 0\n"));
+	unlock_kernel();
 
 	return nonseekable_open(inode, file);
 }
@@ -2369,7 +2374,7 @@ static int cs4297a_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int cs4297a_open(struct inode *inode, struct file *file)
+static int cs4297a_locked_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
 	struct cs4297a_state *s=NULL;
@@ -2486,6 +2491,16 @@ static int cs4297a_open(struct inode *inode, struct file *file)
 	return nonseekable_open(inode, file);
 }
 
+static int cs4297a_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	lock_kernel();
+	ret = cs4297a_open(inode, file);
+	unlock_kernel();
+
+	return ret;
+}
 
 // ******************************************************************************************
 //   Wave (audio) file operations struct.
diff --git a/sound/oss/vwsnd.c b/sound/oss/vwsnd.c
index 20b3b32..99c94c4 100644
--- a/sound/oss/vwsnd.c
+++ b/sound/oss/vwsnd.c
@@ -2921,6 +2921,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	DBGE("(inode=0x%p, file=0x%p)\n", inode, file);
 
+	lock_kernel();
 	INC_USE_COUNT;
 	for (devc = vwsnd_dev_list; devc; devc = devc->next_dev)
 		if ((devc->audio_minor & ~0x0F) == (minor & ~0x0F))
@@ -2928,6 +2929,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	if (devc == NULL) {
 		DEC_USE_COUNT;
+		unlock_kernel();
 		return -ENODEV;
 	}
 
@@ -2936,11 +2938,13 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 		mutex_unlock(&devc->open_mutex);
 		if (file->f_flags & O_NONBLOCK) {
 			DEC_USE_COUNT;
+			unlock_kernel();
 			return -EBUSY;
 		}
 		interruptible_sleep_on(&devc->open_wait);
 		if (signal_pending(current)) {
 			DEC_USE_COUNT;
+			unlock_kernel();
 			return -ERESTARTSYS;
 		}
 		mutex_lock(&devc->open_mutex);
@@ -2993,6 +2997,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	file->private_data = devc;
 	DBGRV();
+	unlock_kernel();
 	return 0;
 }
 
@@ -3062,15 +3067,18 @@ static int vwsnd_mixer_open(struct inode *inode, struct file *file)
 	DBGEV("(inode=0x%p, file=0x%p)\n", inode, file);
 
 	INC_USE_COUNT;
+	lock_kernel();
 	for (devc = vwsnd_dev_list; devc; devc = devc->next_dev)
 		if (devc->mixer_minor == iminor(inode))
 			break;
 
 	if (devc == NULL) {
 		DEC_USE_COUNT;
+		unlock_kernel();
 		return -ENODEV;
 	}
 	file->private_data = devc;
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/sound_core.c b/sound/sound_core.c
index c8627fc..cb61317 100644
--- a/sound/sound_core.c
+++ b/sound/sound_core.c
@@ -629,12 +629,8 @@ static int soundcore_open(struct inode *inode, struct file *file)
 		file->f_op = new_fops;
 		spin_unlock(&sound_loader_lock);
 
-		if (file->f_op->open) {
-			/* TODO: push down BKL into indivial open functions */
-			lock_kernel();
+		if (file->f_op->open)
 			err = file->f_op->open(inode,file);
-			unlock_kernel();
-		}
 
 		if (err) {
 			fops_put(file->f_op);
-- 
1.7.1

Re: [PATCH 3/3] sound: push BKL into open functions

[Jaroslav Kysela]

On Sat, 10 Jul 2010, Arnd Bergmann wrote:

> --- a/sound/core/hwdep.c
> +++ b/sound/core/hwdep.c
> @@ -94,13 +94,18 @@ static int snd_hwdep_open(struct inode *inode, struct file * file)
> 		hw = snd_lookup_oss_minor_data(iminor(inode),
> 					       SNDRV_OSS_DEVICE_TYPE_DMFM);
> #endif
> -	} else
> -		return -ENXIO;
> +	} else {
> +		err = -ENXIO;
> +		goto out;
> +	}
> +
> +	err = -ENODEV;
> 	if (hw == NULL)
> -		return -ENODEV;
> +		goto out;
>
> +	err = -EFAULT;
> 	if (!try_module_get(hw->card->module))
> -		return -EFAULT;
> +		goto out;
>
> 	init_waitqueue_entry(&wait, current);
> 	add_wait_queue(&hw->open_wait, &wait);
> @@ -147,6 +152,7 @@ static int snd_hwdep_open(struct inode *inode, struct file * file)
> 	mutex_unlock(&hw->open_mutex);
> 	if (err < 0)
> 		module_put(hw->card->module);
> +out:
> 	return err;
> }
>
> diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
> index f50ebf2..c30b1f3 100644
> --- a/sound/core/oss/mixer_oss.c
> +++ b/sound/core/oss/mixer_oss.c
> @@ -49,17 +49,19 @@ static int snd_mixer_oss_open(struct inode *inode, struct file *file)
>
> 	card = snd_lookup_oss_minor_data(iminor(inode),
> 					 SNDRV_OSS_DEVICE_TYPE_MIXER);
> -	if (card == NULL)
> -		return -ENODEV;
> -	if (card->mixer_oss == NULL)
> -		return -ENODEV;
> +	err = -ENODEV;
> +	if (card == NULL || card->mixer_oss == NULL)
> +		goto out;
> +
> 	err = snd_card_file_add(card, file);
> 	if (err < 0)
> -		return err;
> +		goto out;
> +
> 	fmixer = kzalloc(sizeof(*fmixer), GFP_KERNEL);
> 	if (fmixer == NULL) {
> 		snd_card_file_remove(card, file);
> -		return -ENOMEM;
> +		err = -ENOMEM;
> +		goto out;
> 	}
> 	fmixer->card = card;
> 	fmixer->mixer = card->mixer_oss;
> @@ -67,9 +69,10 @@ static int snd_mixer_oss_open(struct inode *inode, struct file *file)
> 	if (!try_module_get(card->module)) {
> 		kfree(fmixer);
> 		snd_card_file_remove(card, file);
> -		return -EFAULT;
> +		err = -EFAULT;
> 	}
> -	return 0;
> +out:
> +	return err;
> }
>

I don't see any reason (benefit) to add gotos to these two functions.

 						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.

Re: [PATCH 3/3] sound: push BKL into open functions

[Arnd Bergmann]

This moves the lock_kernel() call from soundcore_open
to the individual OSS device drivers, where we can deal
with it one driver at a time if needed, or just kill
off the drivers.

All core components in ALSA already provide
adequate locking in their open()-functions
and do not require the big kernel lock, so
there is no need to add the BKL there.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
On Sunday 11 July 2010 09:15:22 Jaroslav Kysela wrote:
> I don't see any reason (benefit) to add gotos to these two functions.
> 
Sorry, I had removed them and then forgot the git-add before
creating the emails. Originally, I had two patches, one for
pushing down the BKL into every sound driver (hence the goto)
and a second patch to remove the BKL again from all the native
alsa drivers. If you prefer, I can also give you the separate
patches, but I figured that since none of the ALSA drivers needs
the BKL, the combined patch would be better.

This is the corrected combined version.

	Arnd

 arch/um/drivers/hostaudio_kern.c   |    6 ++++++
 sound/oss/au1550_ac97.c            |   26 +++++++++++++++++---------
 sound/oss/dmasound/dmasound_core.c |   28 ++++++++++++++++++++++------
 sound/oss/msnd_pinnacle.c          |   10 +++++++---
 sound/oss/sh_dac_audio.c           |    9 +++++++--
 sound/oss/soundcard.c              |   20 +++++++++++---------
 sound/oss/swarm_cs4297a.c          |   17 ++++++++++++++++-
 sound/oss/vwsnd.c                  |    8 ++++++++
 sound/sound_core.c                 |    6 +-----
 9 files changed, 95 insertions(+), 35 deletions(-)

diff --git a/arch/um/drivers/hostaudio_kern.c b/arch/um/drivers/hostaudio_kern.c
index ae42695..68142df 100644
--- a/arch/um/drivers/hostaudio_kern.c
+++ b/arch/um/drivers/hostaudio_kern.c
@@ -8,6 +8,7 @@
 #include "linux/slab.h"
 #include "linux/sound.h"
 #include "linux/soundcard.h"
+#include "linux/smp_lock.h"
 #include "asm/uaccess.h"
 #include "init.h"
 #include "os.h"
@@ -198,7 +199,10 @@ static int hostaudio_open(struct inode *inode, struct file *file)
 	if (file->f_mode & FMODE_WRITE)
 		w = 1;
 
+	lock_kernel();
 	ret = os_open_file(dsp, of_set_rw(OPENFLAGS(), r, w), 0);
+	unlock_kernel();
+
 	if (ret < 0) {
 		kfree(state);
 		return ret;
@@ -254,7 +258,9 @@ static int hostmixer_open_mixdev(struct inode *inode, struct file *file)
 	if (file->f_mode & FMODE_WRITE)
 		w = 1;
 
+	lock_kernel();
 	ret = os_open_file(mixer, of_set_rw(OPENFLAGS(), r, w), 0);
+	unlock_kernel();
 
 	if (ret < 0) {
 		printk(KERN_ERR "hostaudio_open_mixdev failed to open '%s', "
diff --git a/sound/oss/au1550_ac97.c b/sound/oss/au1550_ac97.c
index c1070e3..47143a3 100644
--- a/sound/oss/au1550_ac97.c
+++ b/sound/oss/au1550_ac97.c
@@ -43,6 +43,7 @@
 #include <linux/sound.h>
 #include <linux/slab.h>
 #include <linux/soundcard.h>
+#include <linux/smp_lock.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -807,7 +808,9 @@ au1550_llseek(struct file *file, loff_t offset, int origin)
 static int
 au1550_open_mixdev(struct inode *inode, struct file *file)
 {
+	lock_kernel();
 	file->private_data = &au1550_state;
+	unlock_kernel();
 	return 0;
 }
 
@@ -1797,21 +1800,22 @@ au1550_open(struct inode *inode, struct file *file)
 #endif
 
 	file->private_data = s;
+	lock_kernel();
 	/* wait for device to become free */
 	mutex_lock(&s->open_mutex);
 	while (s->open_mode & file->f_mode) {
-		if (file->f_flags & O_NONBLOCK) {
-			mutex_unlock(&s->open_mutex);
-			return -EBUSY;
-		}
+		ret = -EBUSY;
+		if (file->f_flags & O_NONBLOCK)
+			goto out;		
 		add_wait_queue(&s->open_wait, &wait);
 		__set_current_state(TASK_INTERRUPTIBLE);
 		mutex_unlock(&s->open_mutex);
 		schedule();
 		remove_wait_queue(&s->open_wait, &wait);
 		set_current_state(TASK_RUNNING);
+		ret = -ERESTARTSYS;
 		if (signal_pending(current))
-			return -ERESTARTSYS;
+			goto out2;
 		mutex_lock(&s->open_mutex);
 	}
 
@@ -1840,17 +1844,21 @@ au1550_open(struct inode *inode, struct file *file)
 
 	if (file->f_mode & FMODE_READ) {
 		if ((ret = prog_dmabuf_adc(s)))
-			return ret;
+			goto out;
 	}
 	if (file->f_mode & FMODE_WRITE) {
 		if ((ret = prog_dmabuf_dac(s)))
-			return ret;
+			goto out;
 	}
 
 	s->open_mode |= file->f_mode & (FMODE_READ | FMODE_WRITE);
-	mutex_unlock(&s->open_mutex);
 	mutex_init(&s->sem);
-	return 0;
+	ret = 0;
+out:
+	mutex_unlock(&s->open_mutex);
+out2:
+	unlock_kernel();
+	return ret;
 }
 
 static int
diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index 3f3c3f7..5a4f38c 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -323,9 +323,13 @@ static struct {
 
 static int mixer_open(struct inode *inode, struct file *file)
 {
-	if (!try_module_get(dmasound.mach.owner))
+	lock_kernel();
+	if (!try_module_get(dmasound.mach.owner)) {
+		unlock_kernel();
 		return -ENODEV;
+	}
 	mixer.busy = 1;
+	unlock_kernel();
 	return 0;
 }
 
@@ -737,8 +741,11 @@ static int sq_open(struct inode *inode, struct file *file)
 {
 	int rc;
 
-	if (!try_module_get(dmasound.mach.owner))
+	lock_kernel();
+	if (!try_module_get(dmasound.mach.owner)) {
+		unlock_kernel();
 		return -ENODEV;
+	}
 
 	rc = write_sq_open(file); /* checks the f_mode */
 	if (rc)
@@ -781,10 +788,11 @@ static int sq_open(struct inode *inode, struct file *file)
 		sound_set_format(AFMT_MU_LAW);
 	}
 #endif
-
+	unlock_kernel();
 	return 0;
  out:
 	module_put(dmasound.mach.owner);
+	unlock_kernel();
 	return rc;
 }
 
@@ -1226,12 +1234,17 @@ static int state_open(struct inode *inode, struct file *file)
 {
 	char *buffer = state.buf;
 	int len = 0;
+	int ret;
 
+	lock_kernel();
+	ret = -EBUSY;
 	if (state.busy)
-		return -EBUSY;
+		goto out;
 
+	ret = -ENODEV;
 	if (!try_module_get(dmasound.mach.owner))
-		return -ENODEV;
+		goto out;
+
 	state.ptr = 0;
 	state.busy = 1;
 
@@ -1293,7 +1306,10 @@ printk("dmasound: stat buffer used %d bytes\n", len) ;
 		printk(KERN_ERR "dmasound_core: stat buffer overflowed!\n");
 
 	state.len = len;
-	return 0;
+	ret = 0;
+out:
+	unlock_kernel();
+	return ret;
 }
 
 static int state_release(struct inode *inode, struct file *file)
diff --git a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c
index a1e3f96..153d822 100644
--- a/sound/oss/msnd_pinnacle.c
+++ b/sound/oss/msnd_pinnacle.c
@@ -756,12 +756,15 @@ static int dev_open(struct inode *inode, struct file *file)
 	int minor = iminor(inode);
 	int err = 0;
 
+	lock_kernel();
 	if (minor == dev.dsp_minor) {
 		if ((file->f_mode & FMODE_WRITE &&
 		     test_bit(F_AUDIO_WRITE_INUSE, &dev.flags)) ||
 		    (file->f_mode & FMODE_READ &&
-		     test_bit(F_AUDIO_READ_INUSE, &dev.flags)))
-			return -EBUSY;
+		     test_bit(F_AUDIO_READ_INUSE, &dev.flags))) {
+			err = -EBUSY;
+			goto out;
+		}
 
 		if ((err = dsp_open(file)) >= 0) {
 			dev.nresets = 0;
@@ -782,7 +785,8 @@ static int dev_open(struct inode *inode, struct file *file)
 		/* nothing */
 	} else
 		err = -EINVAL;
-
+out:
+	unlock_kernel();
 	return err;
 }
 
diff --git a/sound/oss/sh_dac_audio.c b/sound/oss/sh_dac_audio.c
index 4153752..8f0be40 100644
--- a/sound/oss/sh_dac_audio.c
+++ b/sound/oss/sh_dac_audio.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/fs.h>
 #include <linux/sound.h>
+#include <linux/smp_lock.h>
 #include <linux/soundcard.h>
 #include <linux/interrupt.h>
 #include <linux/hrtimer.h>
@@ -216,13 +217,17 @@ static int dac_audio_open(struct inode *inode, struct file *file)
 {
 	if (file->f_mode & FMODE_READ)
 		return -ENODEV;
-	if (in_use)
+
+	lock_kernel();
+	if (in_use) {
+		unlock_kernel();
 		return -EBUSY;
+	}
 
 	in_use = 1;
 
 	dac_audio_start();
-
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/oss/soundcard.c b/sound/oss/soundcard.c
index 2d9c513..92aa762 100644
--- a/sound/oss/soundcard.c
+++ b/sound/oss/soundcard.c
@@ -210,42 +210,44 @@ static int sound_open(struct inode *inode, struct file *file)
 		printk(KERN_ERR "Invalid minor device %d\n", dev);
 		return -ENXIO;
 	}
+	lock_kernel();
 	switch (dev & 0x0f) {
 	case SND_DEV_CTL:
 		dev >>= 4;
 		if (dev >= 0 && dev < MAX_MIXER_DEV && mixer_devs[dev] == NULL) {
 			request_module("mixer%d", dev);
 		}
+		retval = -ENXIO;
 		if (dev && (dev >= num_mixers || mixer_devs[dev] == NULL))
-			return -ENXIO;
+			break;
 	
 		if (!try_module_get(mixer_devs[dev]->owner))
-			return -ENXIO;
+			break;
+
+		retval = 0;
 		break;
 
 	case SND_DEV_SEQ:
 	case SND_DEV_SEQ2:
-		if ((retval = sequencer_open(dev, file)) < 0)
-			return retval;
+		retval = sequencer_open(dev, file);
 		break;
 
 	case SND_DEV_MIDIN:
-		if ((retval = MIDIbuf_open(dev, file)) < 0)
-			return retval;
+		retval = MIDIbuf_open(dev, file);
 		break;
 
 	case SND_DEV_DSP:
 	case SND_DEV_DSP16:
 	case SND_DEV_AUDIO:
-		if ((retval = audio_open(dev, file)) < 0)
-			return retval;
+		retval = audio_open(dev, file);
 		break;
 
 	default:
 		printk(KERN_ERR "Invalid minor device %d\n", dev);
-		return -ENXIO;
+		retval = -ENXIO;
 	}
 
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/oss/swarm_cs4297a.c b/sound/oss/swarm_cs4297a.c
index 3136c88..34b0838 100644
--- a/sound/oss/swarm_cs4297a.c
+++ b/sound/oss/swarm_cs4297a.c
@@ -68,6 +68,7 @@
 #include <linux/delay.h>
 #include <linux/sound.h>
 #include <linux/slab.h>
+#include <linux/smp_lock.h>
 #include <linux/soundcard.h>
 #include <linux/ac97_codec.h>
 #include <linux/pci.h>
@@ -1534,6 +1535,7 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 	CS_DBGOUT(CS_FUNCTION | CS_OPEN, 4,
 		  printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()+\n"));
 
+	lock_kernel();
 	list_for_each(entry, &cs4297a_devs)
 	{
 		s = list_entry(entry, struct cs4297a_state, list);
@@ -1544,6 +1546,8 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 	{
 		CS_DBGOUT(CS_FUNCTION | CS_OPEN | CS_ERROR, 2,
 			printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()- -ENODEV\n"));
+
+		unlock_kernel();
 		return -ENODEV;
 	}
 	VALIDATE_STATE(s);
@@ -1551,6 +1555,7 @@ static int cs4297a_open_mixdev(struct inode *inode, struct file *file)
 
 	CS_DBGOUT(CS_FUNCTION | CS_OPEN, 4,
 		  printk(KERN_INFO "cs4297a: cs4297a_open_mixdev()- 0\n"));
+	unlock_kernel();
 
 	return nonseekable_open(inode, file);
 }
@@ -2369,7 +2374,7 @@ static int cs4297a_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int cs4297a_open(struct inode *inode, struct file *file)
+static int cs4297a_locked_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
 	struct cs4297a_state *s=NULL;
@@ -2486,6 +2491,16 @@ static int cs4297a_open(struct inode *inode, struct file *file)
 	return nonseekable_open(inode, file);
 }
 
+static int cs4297a_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	lock_kernel();
+	ret = cs4297a_open(inode, file);
+	unlock_kernel();
+
+	return ret;
+}
 
 // ******************************************************************************************
 //   Wave (audio) file operations struct.
diff --git a/sound/oss/vwsnd.c b/sound/oss/vwsnd.c
index 20b3b32..99c94c4 100644
--- a/sound/oss/vwsnd.c
+++ b/sound/oss/vwsnd.c
@@ -2921,6 +2921,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	DBGE("(inode=0x%p, file=0x%p)\n", inode, file);
 
+	lock_kernel();
 	INC_USE_COUNT;
 	for (devc = vwsnd_dev_list; devc; devc = devc->next_dev)
 		if ((devc->audio_minor & ~0x0F) == (minor & ~0x0F))
@@ -2928,6 +2929,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	if (devc == NULL) {
 		DEC_USE_COUNT;
+		unlock_kernel();
 		return -ENODEV;
 	}
 
@@ -2936,11 +2938,13 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 		mutex_unlock(&devc->open_mutex);
 		if (file->f_flags & O_NONBLOCK) {
 			DEC_USE_COUNT;
+			unlock_kernel();
 			return -EBUSY;
 		}
 		interruptible_sleep_on(&devc->open_wait);
 		if (signal_pending(current)) {
 			DEC_USE_COUNT;
+			unlock_kernel();
 			return -ERESTARTSYS;
 		}
 		mutex_lock(&devc->open_mutex);
@@ -2993,6 +2997,7 @@ static int vwsnd_audio_open(struct inode *inode, struct file *file)
 
 	file->private_data = devc;
 	DBGRV();
+	unlock_kernel();
 	return 0;
 }
 
@@ -3062,15 +3067,18 @@ static int vwsnd_mixer_open(struct inode *inode, struct file *file)
 	DBGEV("(inode=0x%p, file=0x%p)\n", inode, file);
 
 	INC_USE_COUNT;
+	lock_kernel();
 	for (devc = vwsnd_dev_list; devc; devc = devc->next_dev)
 		if (devc->mixer_minor == iminor(inode))
 			break;
 
 	if (devc == NULL) {
 		DEC_USE_COUNT;
+		unlock_kernel();
 		return -ENODEV;
 	}
 	file->private_data = devc;
+	unlock_kernel();
 	return 0;
 }
 
diff --git a/sound/sound_core.c b/sound/sound_core.c
index c8627fc..cb61317 100644
--- a/sound/sound_core.c
+++ b/sound/sound_core.c
@@ -629,12 +629,8 @@ static int soundcore_open(struct inode *inode, struct file *file)
 		file->f_op = new_fops;
 		spin_unlock(&sound_loader_lock);
 
-		if (file->f_op->open) {
-			/* TODO: push down BKL into indivial open functions */
-			lock_kernel();
+		if (file->f_op->open)
 			err = file->f_op->open(inode,file);
-			unlock_kernel();
-		}
 
 		if (err) {
 			fops_put(file->f_op);

Re: [PATCH 3/3] sound: push BKL into open functions

[Takashi Iwai]

At Sun, 11 Jul 2010 12:16:36 +0200,
Arnd Bergmann wrote:
> 
> This moves the lock_kernel() call from soundcore_open
> to the individual OSS device drivers, where we can deal
> with it one driver at a time if needed, or just kill
> off the drivers.
> 
> All core components in ALSA already provide
> adequate locking in their open()-functions
> and do not require the big kernel lock, so
> there is no need to add the BKL there.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> On Sunday 11 July 2010 09:15:22 Jaroslav Kysela wrote:
> > I don't see any reason (benefit) to add gotos to these two functions.
> > 
> Sorry, I had removed them and then forgot the git-add before
> creating the emails. Originally, I had two patches, one for
> pushing down the BKL into every sound driver (hence the goto)
> and a second patch to remove the BKL again from all the native
> alsa drivers. If you prefer, I can also give you the separate
> patches, but I figured that since none of the ALSA drivers needs
> the BKL, the combined patch would be better.
> 
> This is the corrected combined version.

Thanks.  I applied now to sound git tree (with a minor fix of
coding-style in sound/oss/au1550_ac97.c).

BTW, do you have an updated patch for native_ioctl conversion wrt
sound/*?


Takashi

[PATCH] sound/oss: convert to unlocked_ioctl

[Arnd Bergmann]

These are the final conversions for the ioctl file operation so we can remove
it in the next merge window.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
---
On Monday 12 July 2010 17:53:32 Takashi Iwai wrote:
> 
> BTW, do you have an updated patch for native_ioctl conversion wrt
> sound/*?

Almost forgot that, thanks for reminding me!

	Arnd
---

 sound/oss/au1550_ac97.c            |   54 ++++++++++++++++++++++-------------
 sound/oss/dmasound/dmasound_core.c |   35 ++++++++++++++++++----
 sound/oss/msnd_pinnacle.c          |   15 ++++++---
 sound/oss/sh_dac_audio.c           |   18 ++++++++++--
 sound/oss/swarm_cs4297a.c          |   24 ++++++++++++---
 sound/oss/vwsnd.c                  |   24 ++++++++-------
 6 files changed, 119 insertions(+), 51 deletions(-)

diff --git a/sound/oss/au1550_ac97.c b/sound/oss/au1550_ac97.c
index fb913e5..0fd256c 100644
--- a/sound/oss/au1550_ac97.c
+++ b/sound/oss/au1550_ac97.c
@@ -827,22 +827,26 @@ mixdev_ioctl(struct ac97_codec *codec, unsigned int cmd,
 	return codec->mixer_ioctl(codec, cmd, arg);
 }
 
-static int
-au1550_ioctl_mixdev(struct inode *inode, struct file *file,
-			       unsigned int cmd, unsigned long arg)
+static long
+au1550_ioctl_mixdev(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct au1550_state *s = (struct au1550_state *)file->private_data;
 	struct ac97_codec *codec = s->codec;
+	int ret;
+
+	lock_kernel();
+	ret = mixdev_ioctl(codec, cmd, arg);
+	unlock_kernel();
 
-	return mixdev_ioctl(codec, cmd, arg);
+	return ret;
 }
 
 static /*const */ struct file_operations au1550_mixer_fops = {
-	owner:THIS_MODULE,
-	llseek:au1550_llseek,
-	ioctl:au1550_ioctl_mixdev,
-	open:au1550_open_mixdev,
-	release:au1550_release_mixdev,
+	.owner		= THIS_MODULE,
+	.llseek		= au1550_llseek,
+	.unlocked_ioctl	= au1550_ioctl_mixdev,
+	.open		= au1550_open_mixdev,
+	.release	= au1550_release_mixdev,
 };
 
 static int
@@ -1346,8 +1350,7 @@ dma_count_done(struct dmabuf *db)
 
 
 static int
-au1550_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
-							unsigned long arg)
+au1550_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct au1550_state *s = (struct au1550_state *)file->private_data;
 	unsigned long   flags;
@@ -1783,6 +1786,17 @@ au1550_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 	return mixdev_ioctl(s->codec, cmd, arg);
 }
 
+static long
+au1550_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = au1550_ioctl(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
 
 static int
 au1550_open(struct inode *inode, struct file *file)
@@ -1893,15 +1907,15 @@ au1550_release(struct inode *inode, struct file *file)
 }
 
 static /*const */ struct file_operations au1550_audio_fops = {
-	owner:		THIS_MODULE,
-	llseek:		au1550_llseek,
-	read:		au1550_read,
-	write:		au1550_write,
-	poll:		au1550_poll,
-	ioctl:		au1550_ioctl,
-	mmap:		au1550_mmap,
-	open:		au1550_open,
-	release:	au1550_release,
+	.owner		= THIS_MODULE,
+	.llseek		= au1550_llseek,
+	.read		= au1550_read,
+	.write		= au1550_write,
+	.poll		= au1550_poll,
+	.unlocked_ioctl	= au1550_unlocked_ioctl,
+	.mmap		= au1550_mmap,
+	.open		= au1550_open,
+	.release	= au1550_release,
 };
 
 MODULE_AUTHOR("Advanced Micro Devices (AMD), dan@embeddededge.com");
diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index 5a4f38c..6ecd41a 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -341,8 +341,8 @@ static int mixer_release(struct inode *inode, struct file *file)
 	unlock_kernel();
 	return 0;
 }
-static int mixer_ioctl(struct inode *inode, struct file *file, u_int cmd,
-		       u_long arg)
+
+static int mixer_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	if (_SIOC_DIR(cmd) & _SIOC_WRITE)
 	    mixer.modify_counter++;
@@ -366,11 +366,22 @@ static int mixer_ioctl(struct inode *inode, struct file *file, u_int cmd,
 	return -EINVAL;
 }
 
+static long mixer_unlocked_ioctl(struct file *file, u_int cmd, u_long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = mixer_ioctl(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static const struct file_operations mixer_fops =
 {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
-	.ioctl		= mixer_ioctl,
+	.unlocked_ioctl	= mixer_unlocked_ioctl,
 	.open		= mixer_open,
 	.release	= mixer_release,
 };
@@ -963,8 +974,7 @@ printk("dmasound_core: tried to set_queue_frags on a locked queue\n") ;
 	return 0 ;
 }
 
-static int sq_ioctl(struct inode *inode, struct file *file, u_int cmd,
-		    u_long arg)
+static int sq_ioctl(struct file *file, u_int cmd, u_long arg)
 {
 	int val, result;
 	u_long fmt;
@@ -1122,18 +1132,29 @@ static int sq_ioctl(struct inode *inode, struct file *file, u_int cmd,
 		return IOCTL_OUT(arg,val);
 
 	default:
-		return mixer_ioctl(inode, file, cmd, arg);
+		return mixer_ioctl(file, cmd, arg);
 	}
 	return -EINVAL;
 }
 
+static long sq_unlocked_ioctl(struct file *file, u_int cmd, u_long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = sq_ioctl(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static const struct file_operations sq_fops =
 {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.write		= sq_write,
 	.poll		= sq_poll,
-	.ioctl		= sq_ioctl,
+	.unlocked_ioctl	= sq_unlocked_ioctl,
 	.open		= sq_open,
 	.release	= sq_release,
 };
diff --git a/sound/oss/msnd_pinnacle.c b/sound/oss/msnd_pinnacle.c
index 153d822..9ffd29f 100644
--- a/sound/oss/msnd_pinnacle.c
+++ b/sound/oss/msnd_pinnacle.c
@@ -639,21 +639,26 @@ static int mixer_ioctl(unsigned int cmd, unsigned long arg)
 	return -EINVAL;
 }
 
-static int dev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+static long dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int minor = iminor(inode);
+	int ret;
 
 	if (cmd == OSS_GETVERSION) {
 		int sound_version = SOUND_VERSION;
 		return put_user(sound_version, (int __user *)arg);
 	}
 
+	ret = -EINVAL;
+
+	lock_kernel();
 	if (minor == dev.dsp_minor)
-		return dsp_ioctl(file, cmd, arg);
+		ret = dsp_ioctl(file, cmd, arg);
 	else if (minor == dev.mixer_minor)
-		return mixer_ioctl(cmd, arg);
+		ret = mixer_ioctl(cmd, arg);
+	unlock_kernel();
 
-	return -EINVAL;
+	return ret;
 }
 
 static void dsp_write_flush(void)
@@ -1109,7 +1114,7 @@ static const struct file_operations dev_fileops = {
 	.owner		= THIS_MODULE,
 	.read		= dev_read,
 	.write		= dev_write,
-	.ioctl		= dev_ioctl,
+	.unlocked_ioctl	= dev_ioctl,
 	.open		= dev_open,
 	.release	= dev_release,
 };
diff --git a/sound/oss/sh_dac_audio.c b/sound/oss/sh_dac_audio.c
index 8f0be40..fdb58eb 100644
--- a/sound/oss/sh_dac_audio.c
+++ b/sound/oss/sh_dac_audio.c
@@ -15,6 +15,7 @@
 #include <linux/linkage.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
+#include <linux/smp_lock.h>
 #include <linux/sound.h>
 #include <linux/smp_lock.h>
 #include <linux/soundcard.h>
@@ -93,7 +94,7 @@ static void dac_audio_set_rate(void)
 	wakeups_per_second = ktime_set(0, 1000000000 / rate);
 }
 
-static int dac_audio_ioctl(struct inode *inode, struct file *file,
+static int dac_audio_ioctl(struct file *file,
 			   unsigned int cmd, unsigned long arg)
 {
 	int val;
@@ -159,6 +160,17 @@ static int dac_audio_ioctl(struct inode *inode, struct file *file,
 	return -EINVAL;
 }
 
+static long dac_audio_unlocked_ioctl(struct file *file, u_int cmd, u_long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = dac_audio_ioctl(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
+
 static ssize_t dac_audio_write(struct file *file, const char *buf, size_t count,
 			       loff_t * ppos)
 {
@@ -242,8 +254,8 @@ static int dac_audio_release(struct inode *inode, struct file *file)
 
 const struct file_operations dac_audio_fops = {
       .read =		dac_audio_read,
-      .write =	dac_audio_write,
-      .ioctl =	dac_audio_ioctl,
+      .write =		dac_audio_write,
+      .unlocked_ioctl =	dac_audio_unlocked_ioctl,
       .open =		dac_audio_open,
       .release =	dac_audio_release,
 };
diff --git a/sound/oss/swarm_cs4297a.c b/sound/oss/swarm_cs4297a.c
index 34b0838..b15840a 100644
--- a/sound/oss/swarm_cs4297a.c
+++ b/sound/oss/swarm_cs4297a.c
@@ -1571,11 +1571,15 @@ static int cs4297a_release_mixdev(struct inode *inode, struct file *file)
 }
 
 
-static int cs4297a_ioctl_mixdev(struct inode *inode, struct file *file,
+static int cs4297a_ioctl_mixdev(struct file *file,
 			       unsigned int cmd, unsigned long arg)
 {
-	return mixer_ioctl((struct cs4297a_state *) file->private_data, cmd,
+	int ret;
+	lock_kernel();
+	ret = mixer_ioctl((struct cs4297a_state *) file->private_data, cmd,
 			   arg);
+	unlock_kernel();
+	return ret;
 }
 
 
@@ -1585,7 +1589,7 @@ static int cs4297a_ioctl_mixdev(struct inode *inode, struct file *file,
 static const struct file_operations cs4297a_mixer_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
-	.ioctl		= cs4297a_ioctl_mixdev,
+	.unlocked_ioctl	= cs4297a_ioctl_mixdev,
 	.open		= cs4297a_open_mixdev,
 	.release	= cs4297a_release_mixdev,
 };
@@ -1949,7 +1953,7 @@ static int cs4297a_mmap(struct file *file, struct vm_area_struct *vma)
 }
 
 
-static int cs4297a_ioctl(struct inode *inode, struct file *file,
+static int cs4297a_ioctl(struct file *file,
 			unsigned int cmd, unsigned long arg)
 {
 	struct cs4297a_state *s =
@@ -2342,6 +2346,16 @@ static int cs4297a_ioctl(struct inode *inode, struct file *file,
 	return mixer_ioctl(s, cmd, arg);
 }
 
+static long cs4297a_unlocked_ioctl(struct file *file, u_int cmd, u_long arg)
+{
+	int ret;
+
+	lock_kernel();
+	ret = cs4297a_ioctl(file, cmd, arg);
+	unlock_kernel();
+
+	return ret;
+}
 
 static int cs4297a_release(struct inode *inode, struct file *file)
 {
@@ -2511,7 +2525,7 @@ static const struct file_operations cs4297a_audio_fops = {
 	.read		= cs4297a_read,
 	.write		= cs4297a_write,
 	.poll		= cs4297a_poll,
-	.ioctl		= cs4297a_ioctl,
+	.unlocked_ioctl	= cs4297a_unlocked_ioctl,
 	.mmap		= cs4297a_mmap,
 	.open		= cs4297a_open,
 	.release	= cs4297a_release,
diff --git a/sound/oss/vwsnd.c b/sound/oss/vwsnd.c
index 99c94c4..8cd73cd 100644
--- a/sound/oss/vwsnd.c
+++ b/sound/oss/vwsnd.c
@@ -2429,8 +2429,7 @@ static unsigned int vwsnd_audio_poll(struct file *file,
 	return mask;
 }
 
-static int vwsnd_audio_do_ioctl(struct inode *inode,
-				struct file *file,
+static int vwsnd_audio_do_ioctl(struct file *file,
 				unsigned int cmd,
 				unsigned long arg)
 {
@@ -2446,8 +2445,8 @@ static int vwsnd_audio_do_ioctl(struct inode *inode,
 	int ival;
 
 	
-	DBGEV("(inode=0x%p, file=0x%p, cmd=0x%x, arg=0x%lx)\n",
-	      inode, file, cmd, arg);
+	DBGEV("(file=0x%p, cmd=0x%x, arg=0x%lx)\n",
+	      file, cmd, arg);
 	switch (cmd) {
 	case OSS_GETVERSION:		/* _SIOR ('M', 118, int) */
 		DBGX("OSS_GETVERSION\n");
@@ -2885,17 +2884,19 @@ static int vwsnd_audio_do_ioctl(struct inode *inode,
 	return -EINVAL;
 }
 
-static int vwsnd_audio_ioctl(struct inode *inode,
-				struct file *file,
+static long vwsnd_audio_ioctl(struct file *file,
 				unsigned int cmd,
 				unsigned long arg)
 {
 	vwsnd_dev_t *devc = (vwsnd_dev_t *) file->private_data;
 	int ret;
 
+	lock_kernel();
 	mutex_lock(&devc->io_mutex);
-	ret = vwsnd_audio_do_ioctl(inode, file, cmd, arg);
+	ret = vwsnd_audio_do_ioctl(file, cmd, arg);
 	mutex_unlock(&devc->io_mutex);
+	unlock_kernel();
+
 	return ret;
 }
 
@@ -3049,7 +3050,7 @@ static const struct file_operations vwsnd_audio_fops = {
 	.read =		vwsnd_audio_read,
 	.write =	vwsnd_audio_write,
 	.poll =		vwsnd_audio_poll,
-	.ioctl =	vwsnd_audio_ioctl,
+	.unlocked_ioctl = vwsnd_audio_ioctl,
 	.mmap =		vwsnd_audio_mmap,
 	.open =		vwsnd_audio_open,
 	.release =	vwsnd_audio_release,
@@ -3211,8 +3212,7 @@ static int mixer_write_ioctl(vwsnd_dev_t *devc, unsigned int nr, void __user *ar
 
 /* This is the ioctl entry to the mixer driver. */
 
-static int vwsnd_mixer_ioctl(struct inode *ioctl,
-			      struct file *file,
+static long vwsnd_mixer_ioctl(struct file *file,
 			      unsigned int cmd,
 			      unsigned long arg)
 {
@@ -3223,6 +3223,7 @@ static int vwsnd_mixer_ioctl(struct inode *ioctl,
 
 	DBGEV("(devc=0x%p, cmd=0x%x, arg=0x%lx)\n", devc, cmd, arg);
 
+	lock_kernel();
 	mutex_lock(&devc->mix_mutex);
 	{
 		if ((cmd & ~nrmask) == MIXER_READ(0))
@@ -3233,13 +3234,14 @@ static int vwsnd_mixer_ioctl(struct inode *ioctl,
 			retval = -EINVAL;
 	}
 	mutex_unlock(&devc->mix_mutex);
+	unlock_kernel();
 	return retval;
 }
 
 static const struct file_operations vwsnd_mixer_fops = {
 	.owner =	THIS_MODULE,
 	.llseek =	no_llseek,
-	.ioctl =	vwsnd_mixer_ioctl,
+	.unlocked_ioctl = vwsnd_mixer_ioctl,
 	.open =		vwsnd_mixer_open,
 	.release =	vwsnd_mixer_release,
 };
-- 
1.7.1

Re: [PATCH] sound/oss: convert to unlocked_ioctl

[Takashi Iwai]

At Mon, 12 Jul 2010 19:53:18 +0200,
Arnd Bergmann wrote:
> 
> These are the final conversions for the ioctl file operation so we can remove
> it in the next merge window.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: alsa-devel@alsa-project.org
> ---
> On Monday 12 July 2010 17:53:32 Takashi Iwai wrote:
> > 
> > BTW, do you have an updated patch for native_ioctl conversion wrt
> > sound/*?
> 
> Almost forgot that, thanks for reminding me!

Thanks, applied now.

Now the rest is eliminating each lock_kernel() in sound/oss/*.c :)


Takashi

Re: [PATCH] sound/oss: convert to unlocked_ioctl

[Arnd Bergmann]

On Monday 12 July 2010 22:38:25 Takashi Iwai wrote:
> Now the rest is eliminating each lock_kernel() in sound/oss/*.c :)

For other files, I've used a script (see below) to do this, it probably
works with the OSS files as well, although I have not tried yet.

Of course, another option for OSS device drivers would be to
remove the entire driver ;). Either way, my feeling is that the
OSS drivers are not stopping anyone from building a kernel without
CONFIG_BKL once we have introduced that symbol and made the drivers
depend on it.

	Arnd

---
#!/bin/bash
file=$1
name=$2
if grep -q lock_kernel ${file} ; then
    if grep -q 'include.*linux.mutex.h' ${file} ; then
            sed -i '/include.*<linux\/smp_lock.h>/d' ${file}
    else
            sed -i 's/include.*<linux\/smp_lock.h>.*$/include <linux\/mutex.h>/g' ${file}
    fi
    sed -i ${file} \
        -e "/^#include.*linux.mutex.h/,$ {
                1,/^\(static\|int\|long\)/ {
                     /^\(static\|int\|long\)/istatic DEFINE_MUTEX(${name}_mutex);

} }"  \
    -e "s/\(un\)*lock_kernel\>[ ]*()/mutex_\1lock(\&${name}_mutex)/g" \
    -e '/[      ]*cycle_kernel_lock();/d'
else
    sed -i -e '/include.*\<smp_lock.h\>/d' ${file}  \
                -e '/cycle_kernel_lock()/d'
fi