[alsa-devel] [PATCH 0/3] further BKL removal
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(-)
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);
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;@@ -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.
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);
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
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, };
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
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
participants (3)
-
Arnd Bergmann -
Jaroslav Kysela -
Takashi Iwai