There is nothing to protect inside nvram_llseek(), the file
offset doesn't need to be protected and nvram_len is only
initialized from an __init path.
It's safe to remove the big kernel lock there.
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: John Kacur <[email protected]>
Cc: Sven-Thorsten Dietrich <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Alessio Igor Bogani <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Greg KH <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/char/generic_nvram.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
Index: linux-2.6-tip/drivers/char/generic_nvram.c
===================================================================
--- linux-2.6-tip.orig/drivers/char/generic_nvram.c
+++ linux-2.6-tip/drivers/char/generic_nvram.c
@@ -19,7 +19,6 @@
#include <linux/miscdevice.h>
#include <linux/fcntl.h>
#include <linux/init.h>
-#include <linux/smp_lock.h>
#include <asm/uaccess.h>
#include <asm/nvram.h>
#ifdef CONFIG_PPC_PMAC
@@ -32,7 +31,6 @@ static ssize_t nvram_len;
static loff_t nvram_llseek(struct file *file, loff_t offset, int origin)
{
- lock_kernel();
switch (origin) {
case 1:
offset += file->f_pos;
@@ -41,12 +39,11 @@ static loff_t nvram_llseek(struct file *
offset += nvram_len;
break;
}
- if (offset < 0) {
- unlock_kernel();
+ if (offset < 0)
return -EINVAL;
- }
+
file->f_pos = offset;
- unlock_kernel();
+
return file->f_pos;
}
On Saturday 10 October 2009, Thomas Gleixner wrote:
> There is nothing to protect inside nvram_llseek(), the file
> offset doesn't need to be protected and nvram_len is only
> initialized from an __init path.
>
> It's safe to remove the big kernel lock there.
>
The generic_nvram driver still uses ->ioctl instead of ->unlocked_ioctl.
I guess it would be helpful to change that in the same series, so we
don't get the BKL back as soon as someone does a pushdown into the
remaining ioctl functions.
Arnd <><
On Sun, Oct 11, 2009 at 09:31:40PM +0200, Arnd Bergmann wrote:
> On Saturday 10 October 2009, Thomas Gleixner wrote:
> > There is nothing to protect inside nvram_llseek(), the file
> > offset doesn't need to be protected and nvram_len is only
> > initialized from an __init path.
> >
> > It's safe to remove the big kernel lock there.
> >
>
> The generic_nvram driver still uses ->ioctl instead of ->unlocked_ioctl.
> I guess it would be helpful to change that in the same series, so we
> don't get the BKL back as soon as someone does a pushdown into the
> remaining ioctl functions.
>
> Arnd <><
Right!
I'll add that in a second patch.
I've completely forgotten this ioctl/unlocked_ioctl thing.
On Sun, Oct 11, 2009 at 11:08:10PM +0200, Frederic Weisbecker wrote:
> On Sun, Oct 11, 2009 at 09:31:40PM +0200, Arnd Bergmann wrote:
> > On Saturday 10 October 2009, Thomas Gleixner wrote:
> > > There is nothing to protect inside nvram_llseek(), the file
> > > offset doesn't need to be protected and nvram_len is only
> > > initialized from an __init path.
> > >
> > > It's safe to remove the big kernel lock there.
> > >
> >
> > The generic_nvram driver still uses ->ioctl instead of ->unlocked_ioctl.
> > I guess it would be helpful to change that in the same series, so we
> > don't get the BKL back as soon as someone does a pushdown into the
> > remaining ioctl functions.
> >
> > Arnd <><
>
>
> Right!
> I'll add that in a second patch.
>
> I've completely forgotten this ioctl/unlocked_ioctl thing.
BTW, I was focusing on the lock_kernel() callsites in the kernel which
are around 626 (I've excluded reiserfs)
Now I'm adding the ioctl() sites too:
git-grep "\.ioctl *=" | grep -P "^\S+\.c" | wc -l
452
Hehe :)
On Sunday 11 October 2009, Frederic Weisbecker wrote:
> Now I'm adding the ioctl() sites too:
>
> git-grep "\.ioctl *=" | grep -P "^\S+\.c" | wc -l
> 452
>
> Hehe :)
Not all of them, fortunately.
There are various *_operations structures that have a .ioctl pointer.
While there are a lot of struct file_operations with a locked .ioctl
operation, stuff like block_device_operations does not hold the
BKL in .ioctl but in .locked_ioctl.
Arnd <><
nvram_ioctl is a bkl locked ioctl, but it can be an unlocked ioctl.
- part is provided by the user
- offset is provided by pmac_get_partition() which is safe as it only
touches nvram_partitions, an array inistialized on __init time and
read-only the rest of the time.
- nvram_sync() only relies on core99_nvram_sync() which checks
is_core_99, nvram_data, nvram_image. Those are variables initialized
on __init time only and their direct values are not touched further.
The rest modifies the nvram image header, protected by nv_lock
already.
So it's safe to call nvram_ioctl without the big kernel lock held.
Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: John Kacur <[email protected]>
Cc: Sven-Thorsten Dietrich <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Alessio Igor Bogani <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Alan Cox <[email protected]>
---
drivers/char/generic_nvram.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
index c49200e..fd448aa 100644
--- a/drivers/char/generic_nvram.c
+++ b/drivers/char/generic_nvram.c
@@ -118,11 +118,11 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
}
const struct file_operations nvram_fops = {
- .owner = THIS_MODULE,
- .llseek = nvram_llseek,
- .read = read_nvram,
- .write = write_nvram,
- .ioctl = nvram_ioctl,
+ .owner = THIS_MODULE,
+ .llseek = nvram_llseek,
+ .read = read_nvram,
+ .write = write_nvram,
+ .unlocked_ioctl = nvram_ioctl,
};
static struct miscdevice nvram_dev = {
--
1.6.2.3
On Sun, Oct 11, 2009 at 11:50:24PM +0200, Arnd Bergmann wrote:
> On Sunday 11 October 2009, Frederic Weisbecker wrote:
> > Now I'm adding the ioctl() sites too:
> >
> > git-grep "\.ioctl *=" | grep -P "^\S+\.c" | wc -l
> > 452
> >
> > Hehe :)
>
> Not all of them, fortunately.
>
> There are various *_operations structures that have a .ioctl pointer.
> While there are a lot of struct file_operations with a locked .ioctl
> operation, stuff like block_device_operations does not hold the
> BKL in .ioctl but in .locked_ioctl.
>
> Arnd <><
Oh right. Thanks for the tip.
On Monday 12 October 2009, Frederic Weisbecker wrote:
> nvram_ioctl is a bkl locked ioctl, but it can be an unlocked ioctl.
>
> - part is provided by the user
> - offset is provided by pmac_get_partition() which is safe as it only
> touches nvram_partitions, an array inistialized on __init time and
> read-only the rest of the time.
> - nvram_sync() only relies on core99_nvram_sync() which checks
> is_core_99, nvram_data, nvram_image. Those are variables initialized
> on __init time only and their direct values are not touched further.
> The rest modifies the nvram image header, protected by nv_lock
> already.
>
> So it's safe to call nvram_ioctl without the big kernel lock held.
>
> Reported-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: John Kacur <[email protected]>
> Cc: Sven-Thorsten Dietrich <[email protected]>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Alessio Igor Bogani <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Alan Cox <[email protected]>
Hmm, I wish I had not started using the gmail MTA for sending out
mail, not that address is public forever.
> drivers/char/generic_nvram.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
> index c49200e..fd448aa 100644
> --- a/drivers/char/generic_nvram.c
> +++ b/drivers/char/generic_nvram.c
> @@ -118,11 +118,11 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
> }
>
> const struct file_operations nvram_fops = {
> - .owner = THIS_MODULE,
> - .llseek = nvram_llseek,
> - .read = read_nvram,
> - .write = write_nvram,
> - .ioctl = nvram_ioctl,
> + .owner = THIS_MODULE,
> + .llseek = nvram_llseek,
> + .read = read_nvram,
> + .write = write_nvram,
> + .unlocked_ioctl = nvram_ioctl,
> };
>
> static struct miscdevice nvram_dev = {
The ioctl and unlocked_ioctl functions have a different signature, so you
need to adapt that, not just rename it.
Not sure if we should do it in the same patch, but this driver should also
have a compat_ioctl method pointing to the same function. The ioctl numbers
in this driver are all 32/64 bit clean, but are not listed in fs/compat_ioctl.c,
so adding a .compat_ioctl pointer is the easiest way to fix 32 bit userland
accessing the device.
Arnd <><
nvram_ioctl is a bkl locked ioctl, but it can be an unlocked ioctl.
- part is provided by the user
- offset is provided by pmac_get_partition() which is safe as it only
touches nvram_partitions, an array inistialized on __init time and
read-only the rest of the time.
- nvram_sync() only relies on core99_nvram_sync() which checks
is_core_99, nvram_data, nvram_image. Those are variables initialized
on __init time only and their direct values are not touched further.
The rest modifies the nvram image header, protected by nv_lock
already.
So it's safe to call nvram_ioctl without the big kernel lock held.
v2: Fix nvram_ioctl to fit unlocked_ioctl signature.
Reported-by: Arnd Bergmann <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: John Kacur <[email protected]>
Cc: Sven-Thorsten Dietrich <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Alessio Igor Bogani <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Alan Cox <[email protected]>
---
drivers/char/generic_nvram.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
index c49200e..866d04d 100644
--- a/drivers/char/generic_nvram.c
+++ b/drivers/char/generic_nvram.c
@@ -85,8 +85,7 @@ static ssize_t write_nvram(struct file *file, const char __user *buf,
return p - buf;
}
-static int nvram_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+static int nvram_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
switch(cmd) {
#ifdef CONFIG_PPC_PMAC
@@ -118,11 +117,11 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
}
const struct file_operations nvram_fops = {
- .owner = THIS_MODULE,
- .llseek = nvram_llseek,
- .read = read_nvram,
- .write = write_nvram,
- .ioctl = nvram_ioctl,
+ .owner = THIS_MODULE,
+ .llseek = nvram_llseek,
+ .read = read_nvram,
+ .write = write_nvram,
+ .unlocked_ioctl = nvram_ioctl,
};
static struct miscdevice nvram_dev = {
--
1.6.2.3
On Mon, Oct 12, 2009 at 12:25:05AM +0200, Arnd Bergmann wrote:
> On Monday 12 October 2009, Frederic Weisbecker wrote:
> > nvram_ioctl is a bkl locked ioctl, but it can be an unlocked ioctl.
> >
> > - part is provided by the user
> > - offset is provided by pmac_get_partition() which is safe as it only
> > touches nvram_partitions, an array inistialized on __init time and
> > read-only the rest of the time.
> > - nvram_sync() only relies on core99_nvram_sync() which checks
> > is_core_99, nvram_data, nvram_image. Those are variables initialized
> > on __init time only and their direct values are not touched further.
> > The rest modifies the nvram image header, protected by nv_lock
> > already.
> >
> > So it's safe to call nvram_ioctl without the big kernel lock held.
> >
> > Reported-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Frederic Weisbecker <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: John Kacur <[email protected]>
> > Cc: Sven-Thorsten Dietrich <[email protected]>
> > Cc: Jonathan Corbet <[email protected]>
> > Cc: Alessio Igor Bogani <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: Alan Cox <[email protected]>
>
> Hmm, I wish I had not started using the gmail MTA for sending out
> mail, not that address is public forever.
Sorry, didn't know about that. I'm then taking Arnd Bergmann <[email protected]>
found in the MAINTAINER file for the v2 patch.
> > drivers/char/generic_nvram.c | 10 +++++-----
> > 1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
> > index c49200e..fd448aa 100644
> > --- a/drivers/char/generic_nvram.c
> > +++ b/drivers/char/generic_nvram.c
> > @@ -118,11 +118,11 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
> > }
> >
> > const struct file_operations nvram_fops = {
> > - .owner = THIS_MODULE,
> > - .llseek = nvram_llseek,
> > - .read = read_nvram,
> > - .write = write_nvram,
> > - .ioctl = nvram_ioctl,
> > + .owner = THIS_MODULE,
> > + .llseek = nvram_llseek,
> > + .read = read_nvram,
> > + .write = write_nvram,
> > + .unlocked_ioctl = nvram_ioctl,
> > };
> >
> > static struct miscdevice nvram_dev = {
>
> The ioctl and unlocked_ioctl functions have a different signature, so you
> need to adapt that, not just rename it.
Hmm, that's the problem with this file I can't event build test. I really
should grab a cross compiler for powerpc targets.
>
> Not sure if we should do it in the same patch, but this driver should also
> have a compat_ioctl method pointing to the same function. The ioctl numbers
> in this driver are all 32/64 bit clean, but are not listed in fs/compat_ioctl.c,
> so adding a .compat_ioctl pointer is the easiest way to fix 32 bit userland
> accessing the device.
>
> Arnd <><
Added in my todolist then, for a saperate patch because it fixes another
issue.
Thanks!
On Monday 12 October 2009, Frederic Weisbecker wrote:
> Sorry, didn't know about that. I'm then taking Arnd Bergmann <[email protected]>
> found in the MAINTAINER file for the v2 patch.
Sure, there's no way you could have known about this. Vger started rejecting
mail from my usual MTA last week, so until I've found a solution for that
I have to use another MTA, but I didn't realize that gmail rewrites the
From: headers...
Arnd <><
On Monday 12 October 2009, Frederic Weisbecker wrote:
> On Sun, Oct 11, 2009 at 11:50:24PM +0200, Arnd Bergmann wrote:
> >
> > There are various *_operations structures that have a .ioctl pointer.
> > While there are a lot of struct file_operations with a locked .ioctl
> > operation, stuff like block_device_operations does not hold the
> > BKL in .ioctl but in .locked_ioctl.
>
> Oh right. Thanks for the tip.
>
FWIW, I've done a grep through the current source tree, this should be
the full list of all .ioctl methods in struct file_operations, a total
of 141 instances in 2.6.32-rc4.
When we do a pushdown of the BKL into these functions, we can kill off
the file operation.
Arnd <><
arch/blackfin/mach-bf561/coreb.c: .ioctl = coreb_ioctl,
arch/cris/arch-v10/drivers/ds1302.c: .ioctl = rtc_ioctl,
arch/cris/arch-v10/drivers/gpio.c: .ioctl = gpio_ioctl,
arch/cris/arch-v10/drivers/i2c.c: .ioctl = i2c_ioctl,
arch/cris/arch-v10/drivers/pcf8563.c: .ioctl = pcf8563_ioctl,
arch/cris/arch-v10/drivers/sync_serial.c: .ioctl = sync_serial_ioctl,
arch/cris/arch-v32/drivers/cryptocop.c: .ioctl = cryptocop_ioctl
arch/cris/arch-v32/drivers/i2c.c: .ioctl = i2c_ioctl,
arch/cris/arch-v32/drivers/mach-a3/gpio.c: .ioctl = gpio_ioctl,
arch/cris/arch-v32/drivers/mach-fs/gpio.c: .ioctl = gpio_ioctl,
arch/cris/arch-v32/drivers/pcf8563.c: .ioctl = pcf8563_ioctl
arch/cris/arch-v32/drivers/sync_serial.c: .ioctl = sync_serial_ioctl,
arch/ia64/kernel/perfmon.c: .ioctl = pfm_ioctl,
arch/ia64/sn/kernel/sn2/sn_hwperf.c: .ioctl = sn_hwperf_ioctl,
arch/m68k/bvme6000/rtc.c: .ioctl = rtc_ioctl,
arch/m68k/mvme16x/rtc.c: .ioctl = rtc_ioctl,
arch/powerpc/kernel/nvram_64.c: .ioctl = dev_nvram_ioctl,
arch/sh/boards/mach-landisk/gio.c: .ioctl = gio_ioctl, /* ioctl */
arch/um/drivers/harddog_kern.c: .ioctl = harddog_ioctl,
arch/um/drivers/hostaudio_kern.c: .ioctl = hostaudio_ioctl,
arch/um/drivers/mmapper_kern.c: .ioctl = mmapper_ioctl,
drivers/block/pktcdvd.c: .ioctl = pkt_ctl_ioctl,
drivers/bluetooth/hci_vhci.c: .ioctl = vhci_ioctl,
drivers/char/apm-emulation.c: .ioctl = apm_ioctl,
drivers/char/applicom.c: .ioctl = ac_ioctl,
drivers/char/ds1620.c: .ioctl = ds1620_ioctl,
drivers/char/dtlk.c: .ioctl = dtlk_ioctl,
drivers/char/nvram.c: .ioctl = nvram_ioctl,
drivers/char/generic_nvram.c: .ioctl = nvram_ioctl,
drivers/char/genrtc.c: .ioctl = gen_rtc_ioctl,
drivers/char/hpet.c: .ioctl = hpet_ioctl,
drivers/char/i8k.c: .ioctl = i8k_ioctl,
drivers/char/ipmi/ipmi_devintf.c: .ioctl = ipmi_ioctl,
drivers/char/ipmi/ipmi_watchdog.c: .ioctl = ipmi_ioctl,
drivers/char/istallion.c: .ioctl = stli_memioctl,
drivers/char/lp.c: .ioctl = lp_ioctl,
drivers/char/nwflash.c: .ioctl = flash_ioctl,
drivers/char/raw.c: .ioctl = raw_ioctl,
drivers/char/sonypi.c: .ioctl = sonypi_misc_ioctl,
drivers/char/stallion.c: .ioctl = stl_memioctl,
drivers/char/toshiba.c: .ioctl = tosh_ioctl,
drivers/gpu/drm/i810/i810_dma.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i810/i810_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i830/i830_dma.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i830/i830_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i915/i915_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/mga/mga_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/r128/r128_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/radeon/radeon_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/radeon/radeon_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/savage/savage_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/sis/sis_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/tdfx/tdfx_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/via/via_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i810/i810_dma.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i810/i810_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i830/i830_dma.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i830/i830_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/i915/i915_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/mga/mga_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/r128/r128_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/radeon/radeon_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/radeon/radeon_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/savage/savage_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/sis/sis_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/tdfx/tdfx_drv.c: .ioctl = drm_ioctl,
drivers/gpu/drm/via/via_drv.c: .ioctl = drm_ioctl,
drivers/hwmon/fschmd.c: .ioctl = watchdog_ioctl,
drivers/ide/ide-tape.c: .ioctl = idetape_chrdev_ioctl,
drivers/input/misc/hp_sdc_rtc.c: .ioctl = hp_sdc_rtc_ioctl,
drivers/isdn/capi/capi.c: .ioctl = capi_ioctl,
drivers/isdn/divert/divert_procfs.c: .ioctl = isdn_divert_ioctl,
drivers/isdn/i4l/isdn_common.c: .ioctl = isdn_ioctl,
drivers/isdn/mISDN/timerdev.c: .ioctl = mISDN_ioctl,
drivers/macintosh/ans-lcd.c: .ioctl = anslcd_ioctl,
drivers/macintosh/nvram.c: .ioctl = nvram_ioctl,
drivers/macintosh/via-pmu.c: .ioctl = pmu_ioctl,
drivers/media/dvb/dvb-core/dmxdev.c: .ioctl = dvb_demux_ioctl,
drivers/media/dvb/dvb-core/dmxdev.c: .ioctl = dvb_dvr_ioctl,
drivers/media/dvb/dvb-core/dvb_ca_en50221.c: .ioctl = dvb_ca_en50221_io_ioctl,
drivers/media/dvb/dvb-core/dvb_net.c: .ioctl = dvb_net_ioctl,
drivers/media/dvb/dvb-core/dvb_frontend.c: .ioctl = dvb_generic_ioctl,
drivers/media/dvb/firewire/firedtv-ci.c: .ioctl = dvb_generic_ioctl,
drivers/media/dvb/ttpci/av7110.c: .ioctl = dvb_generic_ioctl,
drivers/media/dvb/ttpci/av7110_av.c: .ioctl = dvb_generic_ioctl,
drivers/media/dvb/ttpci/av7110_av.c: .ioctl = dvb_generic_ioctl,
drivers/media/dvb/ttpci/av7110_ca.c: .ioctl = dvb_generic_ioctl,
drivers/message/i2o/i2o_config.c: .ioctl = i2o_cfg_ioctl,
drivers/mtd/mtdchar.c: .ioctl = mtd_ioctl,
drivers/net/wan/cosa.c: .ioctl = cosa_chardev_ioctl,
drivers/parisc/eisa_eeprom.c: .ioctl = eisa_eeprom_ioctl,
drivers/pcmcia/pcmcia_ioctl.c: .ioctl = ds_ioctl,
drivers/rtc/rtc-m41t80.c: .ioctl = wdt_ioctl,
drivers/s390/char/tape_char.c: .ioctl = tapechar_ioctl,
drivers/s390/char/vmwatchdog.c: .ioctl = &vmwdt_ioctl,
drivers/sbus/char/openprom.c: .ioctl = openprom_ioctl,
drivers/scsi/3w-9xxx.c: .ioctl = twa_chrdev_ioctl,
drivers/scsi/3w-xxxx.c: .ioctl = tw_chrdev_ioctl,
drivers/scsi/aacraid/linit.c: .ioctl = aac_cfg_ioctl,
drivers/scsi/dpt_i2o.c: .ioctl = adpt_ioctl,
drivers/scsi/gdth.c: .ioctl = gdth_ioctl,
drivers/scsi/megaraid.c: .ioctl = megadev_ioctl,
drivers/scsi/megaraid/megaraid_mm.c: .ioctl = mraid_mm_ioctl,
drivers/scsi/osst.c: .ioctl = osst_ioctl,
drivers/scsi/sg.c: .ioctl = sg_ioctl,
drivers/staging/comedi/comedi_fops.c: .ioctl = comedi_ioctl,
drivers/staging/poch/poch.c: .ioctl = poch_ioctl,
drivers/staging/sep/sep_driver.c: .ioctl = sep_ioctl,
drivers/staging/vme/devices/vme_user.c: .ioctl = vme_user_ioctl,
drivers/usb/core/devio.c: .ioctl = usbdev_ioctl,
drivers/usb/mon/mon_bin.c: .ioctl = mon_bin_ioctl,
fs/autofs/root.c: .ioctl = autofs_root_ioctl,
fs/autofs4/root.c: .ioctl = autofs4_root_ioctl,
fs/coda/pioctl.c: .ioctl = coda_pioctl,
fs/coda/psdev.c: .ioctl = coda_psdev_ioctl,
fs/ecryptfs/file.c: .ioctl = ecryptfs_ioctl,
fs/ecryptfs/file.c: .ioctl = ecryptfs_ioctl,
fs/fat/dir.c: .ioctl = fat_dir_ioctl,
fs/fat/file.c: .ioctl = fat_generic_ioctl,
fs/hfsplus/dir.c: .ioctl = hfsplus_ioctl,
fs/hfsplus/inode.c: .ioctl = hfsplus_ioctl,
fs/ncpfs/dir.c: .ioctl = ncp_ioctl,
fs/ncpfs/file.c: .ioctl = ncp_ioctl,
fs/reiserfs/dir.c: .ioctl = reiserfs_ioctl,
fs/reiserfs/file.c: .ioctl = reiserfs_ioctl,
fs/smbfs/dir.c: .ioctl = smb_ioctl,
fs/smbfs/file.c: .ioctl = smb_ioctl,
fs/udf/dir.c: .ioctl = udf_ioctl,
fs/udf/file.c: .ioctl = udf_ioctl,
net/sunrpc/cache.c: .ioctl = cache_ioctl_procfs, /* for FIONREAD */
net/sunrpc/cache.c: .ioctl = cache_ioctl_pipefs, /* for FIONREAD */
net/sunrpc/rpc_pipe.c: .ioctl = rpc_pipe_ioctl,
sound/oss/dmasound/dmasound_core.c: .ioctl = mixer_ioctl,
sound/oss/dmasound/dmasound_core.c: .ioctl = sq_ioctl,
sound/oss/msnd_pinnacle.c: .ioctl = dev_ioctl,
sound/oss/sh_dac_audio.c: .ioctl = dac_audio_ioctl,
sound/oss/soundcard.c: .ioctl = sound_ioctl,
sound/oss/swarm_cs4297a.c: .ioctl = cs4297a_ioctl_mixdev,
sound/oss/swarm_cs4297a.c: .ioctl = cs4297a_ioctl,
sound/oss/vwsnd.c: .ioctl = vwsnd_audio_ioctl,
sound/oss/vwsnd.c: .ioctl = vwsnd_mixer_ioctl,
On Tue, 13 Oct 2009, Arnd Bergmann wrote:
> On Monday 12 October 2009, Frederic Weisbecker wrote:
> > On Sun, Oct 11, 2009 at 11:50:24PM +0200, Arnd Bergmann wrote:
> > >
> > > There are various *_operations structures that have a .ioctl pointer.
> > > While there are a lot of struct file_operations with a locked .ioctl
> > > operation, stuff like block_device_operations does not hold the
> > > BKL in .ioctl but in .locked_ioctl.
> >
> > Oh right. Thanks for the tip.
> >
>
> FWIW, I've done a grep through the current source tree, this should be
> the full list of all .ioctl methods in struct file_operations, a total
> of 141 instances in 2.6.32-rc4.
>
> When we do a pushdown of the BKL into these functions, we can kill off
> the file operation.
>
> Arnd <><
>
> arch/blackfin/mach-bf561/coreb.c: .ioctl = coreb_ioctl,
That one is scary. The BKL is protecting against parallel ioctl
operations, but the operation on the sys control registers is not
protected against other operations on the same registers in
arch/blackfin/mach-bf561/smp.c. IPI code does not take the BKL
afaict. Mike ?
Thanks,
tglx