* [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl [not found] <20080109005416.039551758@gmail.com> @ 2008-01-09 0:54 ` Kevin Winchester 0 siblings, 0 replies; 10+ messages in thread From: Kevin Winchester @ 2008-01-09 0:54 UTC (permalink / raw) To: David Airlie Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors, Kevin Winchester To: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.sourceforge.net Cc: Andi Kleen <andi@firstfloor.org> Cc: linux-kernel@vger.kernel.org Cc: kernel-janitors@vger.kernel.org The drm drivers in this patch all used drm_ioctl to perform their ioctl calls. The common function is converted to use lock_kernel() and unlock_kernel() and the drivers are converted to use .unlocked_ioctl Signed-off-by: Kevin Winchester <kjwinchester@gmail.com> --- I also noted that in the failed kmalloc case in drm_ioctl(), the function immediately returns -ENOMEM, rather than following the error path that calls atomic_dec(&dev->ioctl_count);. I'm not sure if the ioctl_count is just not important in the -ENOMEM case, or if this is a bug. drivers/char/drm/drmP.h | 3 +-- drivers/char/drm/drm_drv.c | 10 ++++++---- drivers/char/drm/i810_dma.c | 2 +- drivers/char/drm/i810_drv.c | 2 +- drivers/char/drm/i830_dma.c | 2 +- drivers/char/drm/i830_drv.c | 2 +- drivers/char/drm/i915_drv.c | 2 +- drivers/char/drm/mga_drv.c | 2 +- drivers/char/drm/r128_drv.c | 2 +- drivers/char/drm/radeon_drv.c | 2 +- drivers/char/drm/savage_drv.c | 2 +- drivers/char/drm/sis_drv.c | 2 +- drivers/char/drm/tdfx_drv.c | 2 +- drivers/char/drm/via_drv.c | 2 +- 14 files changed, 19 insertions(+), 18 deletions(-) Index: v2.6.24-rc7/drivers/char/drm/drmP.h =================================--- v2.6.24-rc7.orig/drivers/char/drm/drmP.h +++ v2.6.24-rc7/drivers/char/drm/drmP.h @@ -833,8 +833,7 @@ static inline int drm_mtrr_del(int handl /* Driver support (drm_drv.h) */ extern int drm_init(struct drm_driver *driver); extern void drm_exit(struct drm_driver *driver); -extern int drm_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long arg); +extern long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); extern long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); extern int drm_lastclose(struct drm_device *dev); Index: v2.6.24-rc7/drivers/char/drm/drm_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/drm_drv.c +++ v2.6.24-rc7/drivers/char/drm/drm_drv.c @@ -438,7 +438,6 @@ static int drm_version(struct drm_device /** * Called whenever a process performs an ioctl on /dev/drm. * - * \param inode device inode. * \param file_priv DRM file private. * \param cmd command. * \param arg user argument. @@ -447,8 +446,7 @@ static int drm_version(struct drm_device * Looks up the ioctl function in the ::ioctls table, checking for root * previleges if so required, and dispatches to the respective function. */ -int drm_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long arg) +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->head->dev; @@ -458,6 +456,7 @@ int drm_ioctl(struct inode *inode, struc int retcode = -EINVAL; char *kdata = NULL; + lock_kernel(); atomic_inc(&dev->ioctl_count); atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]); ++file_priv->ioctl_count; @@ -494,8 +493,10 @@ int drm_ioctl(struct inode *inode, struc } else { if (cmd & (IOC_IN | IOC_OUT)) { kdata = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL); - if (!kdata) + if (!kdata) { + unlock_kernel(); return -ENOMEM; + } } if (cmd & IOC_IN) { @@ -520,6 +521,7 @@ int drm_ioctl(struct inode *inode, struc atomic_dec(&dev->ioctl_count); if (retcode) DRM_DEBUG("ret = %x\n", retcode); + unlock_kernel(); return retcode; } Index: v2.6.24-rc7/drivers/char/drm/i810_dma.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/i810_dma.c +++ v2.6.24-rc7/drivers/char/drm/i810_dma.c @@ -115,7 +115,7 @@ static int i810_mmap_buffers(struct file static const struct file_operations i810_buffer_fops = { .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = i810_mmap_buffers, .fasync = drm_fasync, }; Index: v2.6.24-rc7/drivers/char/drm/i810_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/i810_drv.c +++ v2.6.24-rc7/drivers/char/drm/i810_drv.c @@ -59,7 +59,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/i830_dma.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/i830_dma.c +++ v2.6.24-rc7/drivers/char/drm/i830_dma.c @@ -117,7 +117,7 @@ static int i830_mmap_buffers(struct file static const struct file_operations i830_buffer_fops = { .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = i830_mmap_buffers, .fasync = drm_fasync, }; Index: v2.6.24-rc7/drivers/char/drm/i830_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/i830_drv.c +++ v2.6.24-rc7/drivers/char/drm/i830_drv.c @@ -70,7 +70,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/i915_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/i915_drv.c +++ v2.6.24-rc7/drivers/char/drm/i915_drv.c @@ -64,7 +64,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/mga_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/mga_drv.c +++ v2.6.24-rc7/drivers/char/drm/mga_drv.c @@ -67,7 +67,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/r128_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/r128_drv.c +++ v2.6.24-rc7/drivers/char/drm/r128_drv.c @@ -62,7 +62,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/radeon_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/radeon_drv.c +++ v2.6.24-rc7/drivers/char/drm/radeon_drv.c @@ -85,7 +85,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/savage_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/savage_drv.c +++ v2.6.24-rc7/drivers/char/drm/savage_drv.c @@ -50,7 +50,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/sis_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/sis_drv.c +++ v2.6.24-rc7/drivers/char/drm/sis_drv.c @@ -80,7 +80,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/tdfx_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/tdfx_drv.c +++ v2.6.24-rc7/drivers/char/drm/tdfx_drv.c @@ -48,7 +48,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/via_drv.c =================================--- v2.6.24-rc7.orig/drivers/char/drm/via_drv.c +++ v2.6.24-rc7/drivers/char/drm/via_drv.c @@ -62,7 +62,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, -- ^ permalink raw reply [flat|nested] 10+ messages in thread
* [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl @ 2008-01-09 0:54 ` Kevin Winchester 0 siblings, 0 replies; 10+ messages in thread From: Kevin Winchester @ 2008-01-09 0:54 UTC (permalink / raw) To: David Airlie Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors, Kevin Winchester [-- Attachment #1: drm_ioctl-unlocked_ioctl.patch --] [-- Type: text/plain, Size: 9361 bytes --] To: David Airlie <airlied@linux.ie> Cc: dri-devel@lists.sourceforge.net Cc: Andi Kleen <andi@firstfloor.org> Cc: linux-kernel@vger.kernel.org Cc: kernel-janitors@vger.kernel.org The drm drivers in this patch all used drm_ioctl to perform their ioctl calls. The common function is converted to use lock_kernel() and unlock_kernel() and the drivers are converted to use .unlocked_ioctl Signed-off-by: Kevin Winchester <kjwinchester@gmail.com> --- I also noted that in the failed kmalloc case in drm_ioctl(), the function immediately returns -ENOMEM, rather than following the error path that calls atomic_dec(&dev->ioctl_count);. I'm not sure if the ioctl_count is just not important in the -ENOMEM case, or if this is a bug. drivers/char/drm/drmP.h | 3 +-- drivers/char/drm/drm_drv.c | 10 ++++++---- drivers/char/drm/i810_dma.c | 2 +- drivers/char/drm/i810_drv.c | 2 +- drivers/char/drm/i830_dma.c | 2 +- drivers/char/drm/i830_drv.c | 2 +- drivers/char/drm/i915_drv.c | 2 +- drivers/char/drm/mga_drv.c | 2 +- drivers/char/drm/r128_drv.c | 2 +- drivers/char/drm/radeon_drv.c | 2 +- drivers/char/drm/savage_drv.c | 2 +- drivers/char/drm/sis_drv.c | 2 +- drivers/char/drm/tdfx_drv.c | 2 +- drivers/char/drm/via_drv.c | 2 +- 14 files changed, 19 insertions(+), 18 deletions(-) Index: v2.6.24-rc7/drivers/char/drm/drmP.h =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/drmP.h +++ v2.6.24-rc7/drivers/char/drm/drmP.h @@ -833,8 +833,7 @@ static inline int drm_mtrr_del(int handl /* Driver support (drm_drv.h) */ extern int drm_init(struct drm_driver *driver); extern void drm_exit(struct drm_driver *driver); -extern int drm_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long arg); +extern long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); extern long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); extern int drm_lastclose(struct drm_device *dev); Index: v2.6.24-rc7/drivers/char/drm/drm_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/drm_drv.c +++ v2.6.24-rc7/drivers/char/drm/drm_drv.c @@ -438,7 +438,6 @@ static int drm_version(struct drm_device /** * Called whenever a process performs an ioctl on /dev/drm. * - * \param inode device inode. * \param file_priv DRM file private. * \param cmd command. * \param arg user argument. @@ -447,8 +446,7 @@ static int drm_version(struct drm_device * Looks up the ioctl function in the ::ioctls table, checking for root * previleges if so required, and dispatches to the respective function. */ -int drm_ioctl(struct inode *inode, struct file *filp, - unsigned int cmd, unsigned long arg) +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->head->dev; @@ -458,6 +456,7 @@ int drm_ioctl(struct inode *inode, struc int retcode = -EINVAL; char *kdata = NULL; + lock_kernel(); atomic_inc(&dev->ioctl_count); atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]); ++file_priv->ioctl_count; @@ -494,8 +493,10 @@ int drm_ioctl(struct inode *inode, struc } else { if (cmd & (IOC_IN | IOC_OUT)) { kdata = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL); - if (!kdata) + if (!kdata) { + unlock_kernel(); return -ENOMEM; + } } if (cmd & IOC_IN) { @@ -520,6 +521,7 @@ int drm_ioctl(struct inode *inode, struc atomic_dec(&dev->ioctl_count); if (retcode) DRM_DEBUG("ret = %x\n", retcode); + unlock_kernel(); return retcode; } Index: v2.6.24-rc7/drivers/char/drm/i810_dma.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/i810_dma.c +++ v2.6.24-rc7/drivers/char/drm/i810_dma.c @@ -115,7 +115,7 @@ static int i810_mmap_buffers(struct file static const struct file_operations i810_buffer_fops = { .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = i810_mmap_buffers, .fasync = drm_fasync, }; Index: v2.6.24-rc7/drivers/char/drm/i810_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/i810_drv.c +++ v2.6.24-rc7/drivers/char/drm/i810_drv.c @@ -59,7 +59,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/i830_dma.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/i830_dma.c +++ v2.6.24-rc7/drivers/char/drm/i830_dma.c @@ -117,7 +117,7 @@ static int i830_mmap_buffers(struct file static const struct file_operations i830_buffer_fops = { .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = i830_mmap_buffers, .fasync = drm_fasync, }; Index: v2.6.24-rc7/drivers/char/drm/i830_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/i830_drv.c +++ v2.6.24-rc7/drivers/char/drm/i830_drv.c @@ -70,7 +70,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/i915_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/i915_drv.c +++ v2.6.24-rc7/drivers/char/drm/i915_drv.c @@ -64,7 +64,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/mga_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/mga_drv.c +++ v2.6.24-rc7/drivers/char/drm/mga_drv.c @@ -67,7 +67,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/r128_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/r128_drv.c +++ v2.6.24-rc7/drivers/char/drm/r128_drv.c @@ -62,7 +62,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/radeon_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/radeon_drv.c +++ v2.6.24-rc7/drivers/char/drm/radeon_drv.c @@ -85,7 +85,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/savage_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/savage_drv.c +++ v2.6.24-rc7/drivers/char/drm/savage_drv.c @@ -50,7 +50,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/sis_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/sis_drv.c +++ v2.6.24-rc7/drivers/char/drm/sis_drv.c @@ -80,7 +80,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/tdfx_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/tdfx_drv.c +++ v2.6.24-rc7/drivers/char/drm/tdfx_drv.c @@ -48,7 +48,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, Index: v2.6.24-rc7/drivers/char/drm/via_drv.c =================================================================== --- v2.6.24-rc7.orig/drivers/char/drm/via_drv.c +++ v2.6.24-rc7/drivers/char/drm/via_drv.c @@ -62,7 +62,7 @@ static struct drm_driver driver = { .owner = THIS_MODULE, .open = drm_open, .release = drm_release, - .ioctl = drm_ioctl, + .unlocked_ioctl = drm_ioctl, .mmap = drm_mmap, .poll = drm_poll, .fasync = drm_fasync, -- ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use 2008-01-09 0:54 ` Kevin Winchester @ 2008-01-09 3:37 ` Dave Airlie -1 siblings, 0 replies; 10+ messages in thread From: Dave Airlie @ 2008-01-09 3:37 UTC (permalink / raw) To: Kevin Winchester; +Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors > The drm drivers in this patch all used drm_ioctl to perform their > ioctl calls. The common function is converted to use lock_kernel() > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > NAK I've started looking at this already in the drm git tree, I'm going to provide both locked and unlocked paths for drivers to choose, as we need to audit the drivers on a per-driver basis, the other option is to provide wrappers in each driver to do the lock/unlock kernel and leave drm_ioctl alone.. I'll take a look kmalloc failure case sounds like a bug though.. Dave. > Signed-off-by: Kevin Winchester <kjwinchester@gmail.com> > > --- > > I also noted that in the failed kmalloc case in drm_ioctl(), the function > immediately returns -ENOMEM, rather than following the error path that > calls atomic_dec(&dev->ioctl_count);. I'm not sure if the ioctl_count > is just not important in the -ENOMEM case, or if this is a bug. > > drivers/char/drm/drmP.h | 3 +-- > drivers/char/drm/drm_drv.c | 10 ++++++---- > drivers/char/drm/i810_dma.c | 2 +- > drivers/char/drm/i810_drv.c | 2 +- > drivers/char/drm/i830_dma.c | 2 +- > drivers/char/drm/i830_drv.c | 2 +- > drivers/char/drm/i915_drv.c | 2 +- > drivers/char/drm/mga_drv.c | 2 +- > drivers/char/drm/r128_drv.c | 2 +- > drivers/char/drm/radeon_drv.c | 2 +- > drivers/char/drm/savage_drv.c | 2 +- > drivers/char/drm/sis_drv.c | 2 +- > drivers/char/drm/tdfx_drv.c | 2 +- > drivers/char/drm/via_drv.c | 2 +- > 14 files changed, 19 insertions(+), 18 deletions(-) > > Index: v2.6.24-rc7/drivers/char/drm/drmP.h > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/drmP.h > +++ v2.6.24-rc7/drivers/char/drm/drmP.h > @@ -833,8 +833,7 @@ static inline int drm_mtrr_del(int handl > /* Driver support (drm_drv.h) */ > extern int drm_init(struct drm_driver *driver); > extern void drm_exit(struct drm_driver *driver); > -extern int drm_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg); > +extern long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); > extern long drm_compat_ioctl(struct file *filp, > unsigned int cmd, unsigned long arg); > extern int drm_lastclose(struct drm_device *dev); > Index: v2.6.24-rc7/drivers/char/drm/drm_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/drm_drv.c > +++ v2.6.24-rc7/drivers/char/drm/drm_drv.c > @@ -438,7 +438,6 @@ static int drm_version(struct drm_device > /** > * Called whenever a process performs an ioctl on /dev/drm. > * > - * \param inode device inode. > * \param file_priv DRM file private. > * \param cmd command. > * \param arg user argument. > @@ -447,8 +446,7 @@ static int drm_version(struct drm_device > * Looks up the ioctl function in the ::ioctls table, checking for root > * previleges if so required, and dispatches to the respective function. > */ > -int drm_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg) > +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > { > struct drm_file *file_priv = filp->private_data; > struct drm_device *dev = file_priv->head->dev; > @@ -458,6 +456,7 @@ int drm_ioctl(struct inode *inode, struc > int retcode = -EINVAL; > char *kdata = NULL; > > + lock_kernel(); > atomic_inc(&dev->ioctl_count); > atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]); > ++file_priv->ioctl_count; > @@ -494,8 +493,10 @@ int drm_ioctl(struct inode *inode, struc > } else { > if (cmd & (IOC_IN | IOC_OUT)) { > kdata = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL); > - if (!kdata) > + if (!kdata) { > + unlock_kernel(); > return -ENOMEM; > + } > } > > if (cmd & IOC_IN) { > @@ -520,6 +521,7 @@ int drm_ioctl(struct inode *inode, struc > atomic_dec(&dev->ioctl_count); > if (retcode) > DRM_DEBUG("ret = %x\n", retcode); > + unlock_kernel(); > return retcode; > } > > Index: v2.6.24-rc7/drivers/char/drm/i810_dma.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/i810_dma.c > +++ v2.6.24-rc7/drivers/char/drm/i810_dma.c > @@ -115,7 +115,7 @@ static int i810_mmap_buffers(struct file > static const struct file_operations i810_buffer_fops = { > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = i810_mmap_buffers, > .fasync = drm_fasync, > }; > Index: v2.6.24-rc7/drivers/char/drm/i810_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/i810_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i810_drv.c > @@ -59,7 +59,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/i830_dma.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/i830_dma.c > +++ v2.6.24-rc7/drivers/char/drm/i830_dma.c > @@ -117,7 +117,7 @@ static int i830_mmap_buffers(struct file > static const struct file_operations i830_buffer_fops = { > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = i830_mmap_buffers, > .fasync = drm_fasync, > }; > Index: v2.6.24-rc7/drivers/char/drm/i830_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/i830_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i830_drv.c > @@ -70,7 +70,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/i915_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/i915_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i915_drv.c > @@ -64,7 +64,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/mga_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/mga_drv.c > +++ v2.6.24-rc7/drivers/char/drm/mga_drv.c > @@ -67,7 +67,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/r128_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/r128_drv.c > +++ v2.6.24-rc7/drivers/char/drm/r128_drv.c > @@ -62,7 +62,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/radeon_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/radeon_drv.c > +++ v2.6.24-rc7/drivers/char/drm/radeon_drv.c > @@ -85,7 +85,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/savage_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/savage_drv.c > +++ v2.6.24-rc7/drivers/char/drm/savage_drv.c > @@ -50,7 +50,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/sis_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/sis_drv.c > +++ v2.6.24-rc7/drivers/char/drm/sis_drv.c > @@ -80,7 +80,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/tdfx_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/tdfx_drv.c > +++ v2.6.24-rc7/drivers/char/drm/tdfx_drv.c > @@ -48,7 +48,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/via_drv.c > =================================> --- v2.6.24-rc7.orig/drivers/char/drm/via_drv.c > +++ v2.6.24-rc7/drivers/char/drm/via_drv.c > @@ -62,7 +62,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl @ 2008-01-09 3:37 ` Dave Airlie 0 siblings, 0 replies; 10+ messages in thread From: Dave Airlie @ 2008-01-09 3:37 UTC (permalink / raw) To: Kevin Winchester; +Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors > The drm drivers in this patch all used drm_ioctl to perform their > ioctl calls. The common function is converted to use lock_kernel() > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > NAK I've started looking at this already in the drm git tree, I'm going to provide both locked and unlocked paths for drivers to choose, as we need to audit the drivers on a per-driver basis, the other option is to provide wrappers in each driver to do the lock/unlock kernel and leave drm_ioctl alone.. I'll take a look kmalloc failure case sounds like a bug though.. Dave. > Signed-off-by: Kevin Winchester <kjwinchester@gmail.com> > > --- > > I also noted that in the failed kmalloc case in drm_ioctl(), the function > immediately returns -ENOMEM, rather than following the error path that > calls atomic_dec(&dev->ioctl_count);. I'm not sure if the ioctl_count > is just not important in the -ENOMEM case, or if this is a bug. > > drivers/char/drm/drmP.h | 3 +-- > drivers/char/drm/drm_drv.c | 10 ++++++---- > drivers/char/drm/i810_dma.c | 2 +- > drivers/char/drm/i810_drv.c | 2 +- > drivers/char/drm/i830_dma.c | 2 +- > drivers/char/drm/i830_drv.c | 2 +- > drivers/char/drm/i915_drv.c | 2 +- > drivers/char/drm/mga_drv.c | 2 +- > drivers/char/drm/r128_drv.c | 2 +- > drivers/char/drm/radeon_drv.c | 2 +- > drivers/char/drm/savage_drv.c | 2 +- > drivers/char/drm/sis_drv.c | 2 +- > drivers/char/drm/tdfx_drv.c | 2 +- > drivers/char/drm/via_drv.c | 2 +- > 14 files changed, 19 insertions(+), 18 deletions(-) > > Index: v2.6.24-rc7/drivers/char/drm/drmP.h > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/drmP.h > +++ v2.6.24-rc7/drivers/char/drm/drmP.h > @@ -833,8 +833,7 @@ static inline int drm_mtrr_del(int handl > /* Driver support (drm_drv.h) */ > extern int drm_init(struct drm_driver *driver); > extern void drm_exit(struct drm_driver *driver); > -extern int drm_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg); > +extern long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg); > extern long drm_compat_ioctl(struct file *filp, > unsigned int cmd, unsigned long arg); > extern int drm_lastclose(struct drm_device *dev); > Index: v2.6.24-rc7/drivers/char/drm/drm_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/drm_drv.c > +++ v2.6.24-rc7/drivers/char/drm/drm_drv.c > @@ -438,7 +438,6 @@ static int drm_version(struct drm_device > /** > * Called whenever a process performs an ioctl on /dev/drm. > * > - * \param inode device inode. > * \param file_priv DRM file private. > * \param cmd command. > * \param arg user argument. > @@ -447,8 +446,7 @@ static int drm_version(struct drm_device > * Looks up the ioctl function in the ::ioctls table, checking for root > * previleges if so required, and dispatches to the respective function. > */ > -int drm_ioctl(struct inode *inode, struct file *filp, > - unsigned int cmd, unsigned long arg) > +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > { > struct drm_file *file_priv = filp->private_data; > struct drm_device *dev = file_priv->head->dev; > @@ -458,6 +456,7 @@ int drm_ioctl(struct inode *inode, struc > int retcode = -EINVAL; > char *kdata = NULL; > > + lock_kernel(); > atomic_inc(&dev->ioctl_count); > atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]); > ++file_priv->ioctl_count; > @@ -494,8 +493,10 @@ int drm_ioctl(struct inode *inode, struc > } else { > if (cmd & (IOC_IN | IOC_OUT)) { > kdata = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL); > - if (!kdata) > + if (!kdata) { > + unlock_kernel(); > return -ENOMEM; > + } > } > > if (cmd & IOC_IN) { > @@ -520,6 +521,7 @@ int drm_ioctl(struct inode *inode, struc > atomic_dec(&dev->ioctl_count); > if (retcode) > DRM_DEBUG("ret = %x\n", retcode); > + unlock_kernel(); > return retcode; > } > > Index: v2.6.24-rc7/drivers/char/drm/i810_dma.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/i810_dma.c > +++ v2.6.24-rc7/drivers/char/drm/i810_dma.c > @@ -115,7 +115,7 @@ static int i810_mmap_buffers(struct file > static const struct file_operations i810_buffer_fops = { > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = i810_mmap_buffers, > .fasync = drm_fasync, > }; > Index: v2.6.24-rc7/drivers/char/drm/i810_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/i810_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i810_drv.c > @@ -59,7 +59,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/i830_dma.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/i830_dma.c > +++ v2.6.24-rc7/drivers/char/drm/i830_dma.c > @@ -117,7 +117,7 @@ static int i830_mmap_buffers(struct file > static const struct file_operations i830_buffer_fops = { > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = i830_mmap_buffers, > .fasync = drm_fasync, > }; > Index: v2.6.24-rc7/drivers/char/drm/i830_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/i830_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i830_drv.c > @@ -70,7 +70,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/i915_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/i915_drv.c > +++ v2.6.24-rc7/drivers/char/drm/i915_drv.c > @@ -64,7 +64,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/mga_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/mga_drv.c > +++ v2.6.24-rc7/drivers/char/drm/mga_drv.c > @@ -67,7 +67,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/r128_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/r128_drv.c > +++ v2.6.24-rc7/drivers/char/drm/r128_drv.c > @@ -62,7 +62,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/radeon_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/radeon_drv.c > +++ v2.6.24-rc7/drivers/char/drm/radeon_drv.c > @@ -85,7 +85,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/savage_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/savage_drv.c > +++ v2.6.24-rc7/drivers/char/drm/savage_drv.c > @@ -50,7 +50,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/sis_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/sis_drv.c > +++ v2.6.24-rc7/drivers/char/drm/sis_drv.c > @@ -80,7 +80,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/tdfx_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/tdfx_drv.c > +++ v2.6.24-rc7/drivers/char/drm/tdfx_drv.c > @@ -48,7 +48,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > Index: v2.6.24-rc7/drivers/char/drm/via_drv.c > =================================================================== > --- v2.6.24-rc7.orig/drivers/char/drm/via_drv.c > +++ v2.6.24-rc7/drivers/char/drm/via_drv.c > @@ -62,7 +62,7 @@ static struct drm_driver driver = { > .owner = THIS_MODULE, > .open = drm_open, > .release = drm_release, > - .ioctl = drm_ioctl, > + .unlocked_ioctl = drm_ioctl, > .mmap = drm_mmap, > .poll = drm_poll, > .fasync = drm_fasync, > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl 2008-01-09 3:37 ` [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl Dave Airlie @ 2008-01-09 4:11 ` Andi Kleen -1 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2008-01-09 4:11 UTC (permalink / raw) To: Dave Airlie Cc: Kevin Winchester, dri-devel, Andi Kleen, linux-kernel, kernel-janitors On Wed, Jan 09, 2008 at 03:37:50AM +0000, Dave Airlie wrote: > > > The drm drivers in this patch all used drm_ioctl to perform their > > ioctl calls. The common function is converted to use lock_kernel() > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > NAK Did you actually read Kevin's patch? > > I've started looking at this already in the drm git tree, I'm going to > provide both locked and unlocked paths for drivers to choose, as we need If you do that you'll exactly need Kevin's patch as a base. -Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl @ 2008-01-09 4:11 ` Andi Kleen 0 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2008-01-09 4:11 UTC (permalink / raw) To: Dave Airlie Cc: Kevin Winchester, dri-devel, Andi Kleen, linux-kernel, kernel-janitors On Wed, Jan 09, 2008 at 03:37:50AM +0000, Dave Airlie wrote: > > > The drm drivers in this patch all used drm_ioctl to perform their > > ioctl calls. The common function is converted to use lock_kernel() > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > NAK Did you actually read Kevin's patch? > > I've started looking at this already in the drm git tree, I'm going to > provide both locked and unlocked paths for drivers to choose, as we need If you do that you'll exactly need Kevin's patch as a base. -Andi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use 2008-01-09 4:11 ` Andi Kleen @ 2008-01-09 4:51 ` Dave Airlie -1 siblings, 0 replies; 10+ messages in thread From: Dave Airlie @ 2008-01-09 4:51 UTC (permalink / raw) To: Andi Kleen; +Cc: Kevin Winchester, dri-devel, linux-kernel, kernel-janitors > On Wed, Jan 09, 2008 at 03:37:50AM +0000, Dave Airlie wrote: > > > > > The drm drivers in this patch all used drm_ioctl to perform their > > > ioctl calls. The common function is converted to use lock_kernel() > > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > > > > NAK > > Did you actually read Kevin's patch? Kevin's patch adds the lock/unlock to drm_ioctl which is exactly what I don't want, I want to have drm_ioctl become drm_unlocked_ioctl, and drm_ioctl to wrap it with the lock/unlocks, then the drivers can all use unlocked_ioctl like Kevins patch pointing to drm_ioctl, and can migrate over to drm_unlocked_ioctl post lock auditing, the new latest i915 driver seems to be fine with unlocked ioctls so far.. Yes I can use Kevin's patch as a base most likely, but it doesn't do what I want yet, and I've already started to do it properly in the drm upstream trees.... Dave. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl @ 2008-01-09 4:51 ` Dave Airlie 0 siblings, 0 replies; 10+ messages in thread From: Dave Airlie @ 2008-01-09 4:51 UTC (permalink / raw) To: Andi Kleen; +Cc: Kevin Winchester, dri-devel, linux-kernel, kernel-janitors > On Wed, Jan 09, 2008 at 03:37:50AM +0000, Dave Airlie wrote: > > > > > The drm drivers in this patch all used drm_ioctl to perform their > > > ioctl calls. The common function is converted to use lock_kernel() > > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > > > > NAK > > Did you actually read Kevin's patch? Kevin's patch adds the lock/unlock to drm_ioctl which is exactly what I don't want, I want to have drm_ioctl become drm_unlocked_ioctl, and drm_ioctl to wrap it with the lock/unlocks, then the drivers can all use unlocked_ioctl like Kevins patch pointing to drm_ioctl, and can migrate over to drm_unlocked_ioctl post lock auditing, the new latest i915 driver seems to be fine with unlocked ioctls so far.. Yes I can use Kevin's patch as a base most likely, but it doesn't do what I want yet, and I've already started to do it properly in the drm upstream trees.... Dave. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl 2008-01-09 3:37 ` [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl Dave Airlie @ 2008-01-09 12:28 ` Kevin Winchester -1 siblings, 0 replies; 10+ messages in thread From: Kevin Winchester @ 2008-01-09 12:28 UTC (permalink / raw) To: Dave Airlie; +Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors On Jan 8, 2008 11:37 PM, Dave Airlie <airlied@linux.ie> wrote: > > > The drm drivers in this patch all used drm_ioctl to perform their > > ioctl calls. The common function is converted to use lock_kernel() > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > NAK > > I've started looking at this already in the drm git tree, I'm going to > provide both locked and unlocked paths for drivers to choose, as we need > to audit the drivers on a per-driver basis, the other option is to provide > wrappers in each driver to do the lock/unlock kernel and leave drm_ioctl > alone.. > > I'll take a look kmalloc failure case sounds like a bug though.. > No problem. I interpreted Andi's request for patches as a way to ensure that people were actively working to remove the BKL from ioctl calls. Since you appear to already have been working towards that end, the patch is not really necessary. -- Kevin Winchester ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl @ 2008-01-09 12:28 ` Kevin Winchester 0 siblings, 0 replies; 10+ messages in thread From: Kevin Winchester @ 2008-01-09 12:28 UTC (permalink / raw) To: Dave Airlie; +Cc: dri-devel, Andi Kleen, linux-kernel, kernel-janitors On Jan 8, 2008 11:37 PM, Dave Airlie <airlied@linux.ie> wrote: > > > The drm drivers in this patch all used drm_ioctl to perform their > > ioctl calls. The common function is converted to use lock_kernel() > > and unlock_kernel() and the drivers are converted to use .unlocked_ioctl > > > > NAK > > I've started looking at this already in the drm git tree, I'm going to > provide both locked and unlocked paths for drivers to choose, as we need > to audit the drivers on a per-driver basis, the other option is to provide > wrappers in each driver to do the lock/unlock kernel and leave drm_ioctl > alone.. > > I'll take a look kmalloc failure case sounds like a bug though.. > No problem. I interpreted Andi's request for patches as a way to ensure that people were actively working to remove the BKL from ioctl calls. Since you appear to already have been working towards that end, the patch is not really necessary. -- Kevin Winchester ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-01-09 12:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080109005416.039551758@gmail.com>
2008-01-09 0:54 ` [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl Kevin Winchester
2008-01-09 0:54 ` Kevin Winchester
2008-01-09 3:37 ` [patch 1/1] Convert drivers in drivers/char/drm to use Dave Airlie
2008-01-09 3:37 ` [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl Dave Airlie
2008-01-09 4:11 ` Andi Kleen
2008-01-09 4:11 ` Andi Kleen
2008-01-09 4:51 ` [patch 1/1] Convert drivers in drivers/char/drm to use Dave Airlie
2008-01-09 4:51 ` [patch 1/1] Convert drivers in drivers/char/drm to use .unlocked_ioctl Dave Airlie
2008-01-09 12:28 ` Kevin Winchester
2008-01-09 12:28 ` Kevin Winchester
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.