From e53677113e32e6f118e31b8391a2eab7ee52c0a7 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Sat, 18 Oct 2008 20:27:51 -0700 Subject: fb: push down the BKL in the ioctl handler Framebuffer is heavily BKL dependant at the moment so just wrap the ioctl handler in the driver as we push down. [akpm@linux-foundation.org: coding-style fixes] Signed-off-by: Alan Cox Cc: Krzysztof Helt Cc: "Antonino A. Daplas" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/video/fbmem.c | 141 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 86 insertions(+), 55 deletions(-) (limited to 'drivers/video/fbmem.c') diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 217c5118ae9..2a0f013dc37 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -1002,101 +1002,132 @@ fb_blank(struct fb_info *info, int blank) return ret; } -static int -fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd, +static long +fb_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { + struct inode *inode = file->f_path.dentry->d_inode; int fbidx = iminor(inode); - struct fb_info *info = registered_fb[fbidx]; - struct fb_ops *fb = info->fbops; + struct fb_info *info; + struct fb_ops *fb; struct fb_var_screeninfo var; struct fb_fix_screeninfo fix; struct fb_con2fbmap con2fb; struct fb_cmap_user cmap; struct fb_event event; void __user *argp = (void __user *)arg; - int i; - - if (!fb) + long ret = 0; + + lock_kernel(); + info = registered_fb[fbidx]; + fb = info->fbops; + + if (!fb) { + unlock_kernel(); return -ENODEV; + } switch (cmd) { case FBIOGET_VSCREENINFO: - return copy_to_user(argp, &info->var, + ret = copy_to_user(argp, &info->var, sizeof(var)) ? -EFAULT : 0; + break; case FBIOPUT_VSCREENINFO: - if (copy_from_user(&var, argp, sizeof(var))) - return -EFAULT; + if (copy_from_user(&var, argp, sizeof(var))) { + ret = -EFAULT; + break; + } acquire_console_sem(); info->flags |= FBINFO_MISC_USEREVENT; - i = fb_set_var(info, &var); + ret = fb_set_var(info, &var); info->flags &= ~FBINFO_MISC_USEREVENT; release_console_sem(); - if (i) return i; - if (copy_to_user(argp, &var, sizeof(var))) - return -EFAULT; - return 0; + if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) + ret = -EFAULT; + break; case FBIOGET_FSCREENINFO: - return copy_to_user(argp, &info->fix, + ret = copy_to_user(argp, &info->fix, sizeof(fix)) ? -EFAULT : 0; + break; case FBIOPUTCMAP: if (copy_from_user(&cmap, argp, sizeof(cmap))) - return -EFAULT; - return (fb_set_user_cmap(&cmap, info)); + ret = -EFAULT; + else + ret = fb_set_user_cmap(&cmap, info); + break; case FBIOGETCMAP: if (copy_from_user(&cmap, argp, sizeof(cmap))) - return -EFAULT; - return fb_cmap_to_user(&info->cmap, &cmap); + ret = -EFAULT; + else + ret = fb_cmap_to_user(&info->cmap, &cmap); + break; case FBIOPAN_DISPLAY: - if (copy_from_user(&var, argp, sizeof(var))) - return -EFAULT; + if (copy_from_user(&var, argp, sizeof(var))) { + ret = -EFAULT; + break; + } acquire_console_sem(); - i = fb_pan_display(info, &var); + ret = fb_pan_display(info, &var); release_console_sem(); - if (i) - return i; - if (copy_to_user(argp, &var, sizeof(var))) - return -EFAULT; - return 0; + if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) + ret = -EFAULT; + break; case FBIO_CURSOR: - return -EINVAL; + ret = -EINVAL; + break; case FBIOGET_CON2FBMAP: if (copy_from_user(&con2fb, argp, sizeof(con2fb))) - return -EFAULT; - if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) - return -EINVAL; - con2fb.framebuffer = -1; - event.info = info; - event.data = &con2fb; - fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, &event); - return copy_to_user(argp, &con2fb, + ret = -EFAULT; + else if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) + ret = -EINVAL; + else { + con2fb.framebuffer = -1; + event.info = info; + event.data = &con2fb; + fb_notifier_call_chain(FB_EVENT_GET_CONSOLE_MAP, + &event); + ret = copy_to_user(argp, &con2fb, sizeof(con2fb)) ? -EFAULT : 0; + } + break; case FBIOPUT_CON2FBMAP: - if (copy_from_user(&con2fb, argp, sizeof(con2fb))) - return - EFAULT; - if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) - return -EINVAL; - if (con2fb.framebuffer < 0 || con2fb.framebuffer >= FB_MAX) - return -EINVAL; - if (!registered_fb[con2fb.framebuffer]) - request_module("fb%d", con2fb.framebuffer); + if (copy_from_user(&con2fb, argp, sizeof(con2fb))) { + ret = -EFAULT; + break; + } + if (con2fb.console < 1 || con2fb.console > MAX_NR_CONSOLES) { + ret = -EINVAL; + break; + } + if (con2fb.framebuffer < 0 || con2fb.framebuffer >= FB_MAX) { + ret = -EINVAL; + break; + } if (!registered_fb[con2fb.framebuffer]) - return -EINVAL; + request_module("fb%d", con2fb.framebuffer); + if (!registered_fb[con2fb.framebuffer]) { + ret = -EINVAL; + break; + } event.info = info; event.data = &con2fb; - return fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, + ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event); + break; case FBIOBLANK: acquire_console_sem(); info->flags |= FBINFO_MISC_USEREVENT; - i = fb_blank(info, arg); + ret = fb_blank(info, arg); info->flags &= ~FBINFO_MISC_USEREVENT; release_console_sem(); - return i; + break;; default: if (fb->fb_ioctl == NULL) - return -EINVAL; - return fb->fb_ioctl(info, cmd, arg); + ret = -ENOTTY; + else + ret = fb->fb_ioctl(info, cmd, arg); } + unlock_kernel(); + return ret; } #ifdef CONFIG_COMPAT @@ -1150,7 +1181,7 @@ static int fb_getput_cmap(struct inode *inode, struct file *file, put_user(compat_ptr(data), &cmap->transp)) return -EFAULT; - err = fb_ioctl(inode, file, cmd, (unsigned long) cmap); + err = fb_ioctl(file, cmd, (unsigned long) cmap); if (!err) { if (copy_in_user(&cmap32->start, @@ -1204,7 +1235,7 @@ static int fb_get_fscreeninfo(struct inode *inode, struct file *file, old_fs = get_fs(); set_fs(KERNEL_DS); - err = fb_ioctl(inode, file, cmd, (unsigned long) &fix); + err = fb_ioctl(file, cmd, (unsigned long) &fix); set_fs(old_fs); if (!err) @@ -1231,7 +1262,7 @@ fb_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case FBIOPUT_CON2FBMAP: arg = (unsigned long) compat_ptr(arg); case FBIOBLANK: - ret = fb_ioctl(inode, file, cmd, arg); + ret = fb_ioctl(file, cmd, arg); break; case FBIOGET_FSCREENINFO: @@ -1358,7 +1389,7 @@ static const struct file_operations fb_fops = { .owner = THIS_MODULE, .read = fb_read, .write = fb_write, - .ioctl = fb_ioctl, + .unlocked_ioctl = fb_ioctl, #ifdef CONFIG_COMPAT .compat_ioctl = fb_compat_ioctl, #endif -- cgit v1.2.3 From 3e680aae4e53ab54cdbb0c29257dae0cbb158e1c Mon Sep 17 00:00:00 2001 From: Krzysztof Helt Date: Sat, 18 Oct 2008 20:27:51 -0700 Subject: fb: convert lock/unlock_kernel() into local fb mutex Change lock_kernel()/unlock_kernel() to local fb mutex. Each frame buffer instance has its own mutex. The one line try_to_load() function is unrolled to request_module() in two places for readability. [righi.andrea@gmail.com: fb: fix NULL pointer BUG dereference in fb_open()] Signed-off-by: Krzysztof Helt Signed-off-by: Andrea Righi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- drivers/video/fbmem.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) (limited to 'drivers/video/fbmem.c') diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index 2a0f013dc37..cd5f20da738 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -1018,12 +1018,12 @@ fb_ioctl(struct file *file, unsigned int cmd, void __user *argp = (void __user *)arg; long ret = 0; - lock_kernel(); info = registered_fb[fbidx]; + mutex_lock(&info->lock); fb = info->fbops; if (!fb) { - unlock_kernel(); + mutex_unlock(&info->lock); return -ENODEV; } switch (cmd) { @@ -1126,7 +1126,7 @@ fb_ioctl(struct file *file, unsigned int cmd, else ret = fb->fb_ioctl(info, cmd, arg); } - unlock_kernel(); + mutex_unlock(&info->lock); return ret; } @@ -1253,7 +1253,7 @@ fb_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) struct fb_ops *fb = info->fbops; long ret = -ENOIOCTLCMD; - lock_kernel(); + mutex_lock(&info->lock); switch(cmd) { case FBIOGET_VSCREENINFO: case FBIOPUT_VSCREENINFO: @@ -1279,7 +1279,7 @@ fb_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ret = fb->fb_compat_ioctl(info, cmd, arg); break; } - unlock_kernel(); + mutex_unlock(&info->lock); return ret; } #endif @@ -1301,13 +1301,13 @@ fb_mmap(struct file *file, struct vm_area_struct * vma) return -ENODEV; if (fb->fb_mmap) { int res; - lock_kernel(); + mutex_lock(&info->lock); res = fb->fb_mmap(info, vma); - unlock_kernel(); + mutex_unlock(&info->lock); return res; } - lock_kernel(); + mutex_lock(&info->lock); /* frame buffer memory */ start = info->fix.smem_start; @@ -1316,13 +1316,13 @@ fb_mmap(struct file *file, struct vm_area_struct * vma) /* memory mapped io */ off -= len; if (info->var.accel_flags) { - unlock_kernel(); + mutex_unlock(&info->lock); return -EINVAL; } start = info->fix.mmio_start; len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len); } - unlock_kernel(); + mutex_unlock(&info->lock); start &= PAGE_MASK; if ((vma->vm_end - vma->vm_start + off) > len) return -EINVAL; @@ -1346,13 +1346,13 @@ fb_open(struct inode *inode, struct file *file) if (fbidx >= FB_MAX) return -ENODEV; - lock_kernel(); - if (!(info = registered_fb[fbidx])) + info = registered_fb[fbidx]; + if (!info) request_module("fb%d", fbidx); - if (!(info = registered_fb[fbidx])) { - res = -ENODEV; - goto out; - } + info = registered_fb[fbidx]; + if (!info) + return -ENODEV; + mutex_lock(&info->lock); if (!try_module_get(info->fbops->owner)) { res = -ENODEV; goto out; @@ -1368,7 +1368,7 @@ fb_open(struct inode *inode, struct file *file) fb_deferred_io_open(info, inode, file); #endif out: - unlock_kernel(); + mutex_unlock(&info->lock); return res; } @@ -1377,11 +1377,11 @@ fb_release(struct inode *inode, struct file *file) { struct fb_info * const info = file->private_data; - lock_kernel(); + mutex_lock(&info->lock); if (info->fbops->fb_release) info->fbops->fb_release(info,1); module_put(info->fbops->owner); - unlock_kernel(); + mutex_unlock(&info->lock); return 0; } @@ -1460,6 +1460,7 @@ register_framebuffer(struct fb_info *fb_info) if (!registered_fb[i]) break; fb_info->node = i; + mutex_init(&fb_info->lock); fb_info->dev = device_create(fb_class, fb_info->device, MKDEV(FB_MAJOR, i), NULL, "fb%d", i); -- cgit v1.2.3