* [PATCH] spi: Push the BKL down into the drivers @ 2008-05-22 21:45 Alan Cox 2008-05-23 0:40 ` David Brownell 0 siblings, 1 reply; 3+ messages in thread From: Alan Cox @ 2008-05-22 21:45 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, dbrownell-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f Another step to removing ->ioctl and to removing the BKL Signed-off-by: Alan Cox <alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index b3518ca..67e907c 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -29,6 +29,7 @@ #include <linux/errno.h> #include <linux/mutex.h> #include <linux/slab.h> +#include <linux/smp_lock.h> #include <linux/spi/spi.h> #include <linux/spi/spidev.h> @@ -240,9 +241,8 @@ done: return status; } -static int -spidev_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long arg) +static long +spidev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { int err = 0; int retval = 0; @@ -269,6 +269,7 @@ spidev_ioctl(struct inode *inode, struct file *filp, if (err) return -EFAULT; + lock_kernel(); spidev = filp->private_data; spi = spidev->spi; @@ -357,7 +358,8 @@ spidev_ioctl(struct inode *inode, struct file *filp, /* segmented and/or full-duplex I/O request */ if (_IOC_NR(cmd) != _IOC_NR(SPI_IOC_MESSAGE(0)) || _IOC_DIR(cmd) != _IOC_WRITE) - return -ENOTTY; + retval = -ENOTTY; + break; tmp = _IOC_SIZE(cmd); if ((tmp % sizeof(struct spi_ioc_transfer)) != 0) { @@ -385,6 +387,7 @@ spidev_ioctl(struct inode *inode, struct file *filp, kfree(ioc); break; } + unlock_kernel(); return retval; } @@ -447,7 +450,7 @@ static struct file_operations spidev_fops = { */ .write = spidev_write, .read = spidev_read, - .ioctl = spidev_ioctl, + .unlocked_ioctl = spidev_ioctl, .open = spidev_open, .release = spidev_release, }; ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] spi: Push the BKL down into the drivers 2008-05-22 21:45 [PATCH] spi: Push the BKL down into the drivers Alan Cox @ 2008-05-23 0:40 ` David Brownell [not found] ` <200805221740.51016.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 0 siblings, 1 reply; 3+ messages in thread From: David Brownell @ 2008-05-23 0:40 UTC (permalink / raw) To: Alan Cox Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thursday 22 May 2008, Alan Cox wrote: > Another step to removing ->ioctl and to removing the BKL This happens to not apply with the latest patches; I'll address that. Where is the writeup about exactly what lock_kernel() is supposed to have been protecting against in ioctl code? - Dave ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <200805221740.51016.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>]
* Re: [PATCH] spi: Push the BKL down into the drivers [not found] ` <200805221740.51016.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> @ 2008-05-23 11:09 ` Alan Cox 0 siblings, 0 replies; 3+ messages in thread From: Alan Cox @ 2008-05-23 11:09 UTC (permalink / raw) To: David Brownell Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thu, 22 May 2008 17:40:50 -0700 David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote: > On Thursday 22 May 2008, Alan Cox wrote: > > Another step to removing ->ioctl and to removing the BKL > > This happens to not apply with the latest patches; I'll > address that. > > Where is the writeup about exactly what lock_kernel() is > supposed to have been protecting against in ioctl code? There isn't one. The big problem we have is that the ioctl methods were implicitly called under the big kernel lock. Without pushing them down into drivers we can't take the step of actually thinking at the driver level "is this needed". Historically therefore we've never documented *why* or *if* it was needed - it's just there. Pushing them into the driver means the subsystem owner gets to see the implicit locking and can actually ask the right questions with internal driver knowledge. Whether it is needed is dependant on the internal locking model of the driver - does it handle parallel ioctls correctly, are there races against other functions in the driver (eg hot unplugs). That can't be evaluated at a high level. It's also made harder by the fact lots of drivers have buggy assumptions about ioctl locking (watchdog was a fine example). Alan ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-05-23 11:09 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-05-22 21:45 [PATCH] spi: Push the BKL down into the drivers Alan Cox 2008-05-23 0:40 ` David Brownell [not found] ` <200805221740.51016.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> 2008-05-23 11:09 ` Alan Cox
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).