All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thiago Galesi <thiagogalesi@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fbdev-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fb: Remove use of lock_kernel / unlock_kernel in fbmem
Date: Wed, 16 Apr 2008 20:00:59 -0300	[thread overview]
Message-ID: <200804162000.59587.thiagogalesi@gmail.com> (raw)
In-Reply-To: <20080415221707.82da3d42.akpm@linux-foundation.org>





This patch removes lock_kernel(), unlock_kernel() usage in fbmem.c and replaces it with a mutex

Signed-off-by: Thiago Galesi <thiagogalesi@gmail.com>

---

Ok, 2nd try

The only thing I don't like is the name of fb_ioctl, but I don't have a better idea, so...


---

Index: linux-2.6.23.1/drivers/video/fbmem.c
===================================================================
--- linux-2.6.23.1.orig/drivers/video/fbmem.c
+++ linux-2.6.23.1/drivers/video/fbmem.c
@@ -987,7 +987,7 @@ fb_blank(struct fb_info *info, int blank
 }
 
 static int 
-fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
+__fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 	 unsigned long arg)
 {
 	int fbidx = iminor(inode);
@@ -1085,6 +1085,28 @@ fb_ioctl(struct inode *inode, struct fil
 	}
 }
 
+static int
+fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
+	 unsigned long arg)
+{
+	int fbidx = iminor(inode);
+	struct fb_info *info = registered_fb[fbidx];
+	int ret;
+
+	mutex_lock(&info->hwlock);
+	ret = __fb_ioctl(inode, file, cmd, arg);
+	mutex_unlock(&info->hwlock);
+	return ret;
+}
+
+static long
+fb_unlocked_ioctl(struct file *file, unsigned int cmd,
+	 unsigned long arg)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	return fb_ioctl(inode, file, cmd, arg);
+}
+
 #ifdef CONFIG_COMPAT
 struct fb_fix_screeninfo32 {
 	char			id[16];
@@ -1136,7 +1158,7 @@ static int fb_getput_cmap(struct inode *
 	    put_user(compat_ptr(data), &cmap->transp))
 		return -EFAULT;
 
-	err = fb_ioctl(inode, file, cmd, (unsigned long) cmap);
+	err = __fb_ioctl(inode, file, cmd, (unsigned long) cmap);
 
 	if (!err) {
 		if (copy_in_user(&cmap32->start,
@@ -1190,7 +1212,7 @@ static int fb_get_fscreeninfo(struct ino
 
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
-	err = fb_ioctl(inode, file, cmd, (unsigned long) &fix);
+	err = __fb_ioctl(inode, file, cmd, (unsigned long) &fix);
 	set_fs(old_fs);
 
 	if (!err)
@@ -1208,7 +1230,7 @@ fb_compat_ioctl(struct file *file, unsig
 	struct fb_ops *fb = info->fbops;
 	long ret = -ENOIOCTLCMD;
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 	switch(cmd) {
 	case FBIOGET_VSCREENINFO:
 	case FBIOPUT_VSCREENINFO:
@@ -1217,7 +1239,7 @@ fb_compat_ioctl(struct file *file, unsig
 	case FBIOPUT_CON2FBMAP:
 		arg = (unsigned long) compat_ptr(arg);
 	case FBIOBLANK:
-		ret = fb_ioctl(inode, file, cmd, arg);
+		ret = __fb_ioctl(inode, file, cmd, arg);
 		break;
 
 	case FBIOGET_FSCREENINFO:
@@ -1234,7 +1256,7 @@ fb_compat_ioctl(struct file *file, unsig
 			ret = fb->fb_compat_ioctl(info, cmd, arg);
 		break;
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return ret;
 }
 #endif
@@ -1256,13 +1278,13 @@ fb_mmap(struct file *file, struct vm_are
 		return -ENODEV;
 	if (fb->fb_mmap) {
 		int res;
-		lock_kernel();
+		mutex_lock(&info->hwlock);
 		res = fb->fb_mmap(info, vma);
-		unlock_kernel();
+		mutex_unlock(&info->hwlock);
 		return res;
 	}
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 
 	/* frame buffer memory */
 	start = info->fix.smem_start;
@@ -1271,13 +1293,13 @@ fb_mmap(struct file *file, struct vm_are
 		/* memory mapped io */
 		off -= len;
 		if (info->var.accel_flags) {
-			unlock_kernel();
+			mutex_unlock(&info->hwlock);
 			return -EINVAL;
 		}
 		start = info->fix.mmio_start;
 		len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	start &= PAGE_MASK;
 	if ((vma->vm_end - vma->vm_start + off) > len)
 		return -EINVAL;
@@ -1323,25 +1345,25 @@ fb_release(struct inode *inode, struct f
 {
 	struct fb_info * const info = file->private_data;
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 	if (info->fbops->fb_release)
 		info->fbops->fb_release(info,1);
 	module_put(info->fbops->owner);
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return 0;
 }
 
 static const struct file_operations fb_fops = {
-	.owner =	THIS_MODULE,
-	.read =		fb_read,
-	.write =	fb_write,
-	.ioctl =	fb_ioctl,
+	.owner =		THIS_MODULE,
+	.read =			fb_read,
+	.write =		fb_write,
+	.unlocked_ioctl =	fb_unlocked_ioctl,
 #ifdef CONFIG_COMPAT
-	.compat_ioctl = fb_compat_ioctl,
+	.compat_ioctl = 	fb_compat_ioctl,
 #endif
-	.mmap =		fb_mmap,
-	.open =		fb_open,
-	.release =	fb_release,
+	.mmap =			fb_mmap,
+	.open =			fb_open,
+	.release =		fb_release,
 #ifdef HAVE_ARCH_FB_UNMAPPED_AREA
 	.get_unmapped_area = get_fb_unmapped_area,
 #endif
@@ -1413,6 +1435,8 @@ register_framebuffer(struct fb_info *fb_
 
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+
+	mutex_init(&fb_info->hwlock);
 	return 0;
 }
 
@@ -1464,6 +1488,7 @@ unregister_framebuffer(struct fb_info *f
 	device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
+	mutex_destroy(&fb_info.hwlock);
 done:
 	return ret;
 }
Index: linux-2.6.23.1/include/linux/fb.h
===================================================================
--- linux-2.6.23.1.orig/include/linux/fb.h
+++ linux-2.6.23.1/include/linux/fb.h
@@ -802,6 +802,7 @@ struct fb_info {
 	struct list_head modelist;      /* mode list */
 	struct fb_videomode *mode;	/* current mode */
 
+	struct mutex hwlock;		/* mutex for protecting hw ops */
 #ifdef CONFIG_FB_BACKLIGHT
 	/* assigned backlight device */
 	/* set before framebuffer registration, 







-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

WARNING: multiple messages have this Message-ID (diff)
From: Thiago Galesi <thiagogalesi@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-fbdev-devel@lists.sourceforge.net
Subject: Re: [Linux-fbdev-devel] [PATCH] fb: Remove use of lock_kernel / unlock_kernel in fbmem
Date: Wed, 16 Apr 2008 20:00:59 -0300	[thread overview]
Message-ID: <200804162000.59587.thiagogalesi@gmail.com> (raw)
In-Reply-To: <20080415221707.82da3d42.akpm@linux-foundation.org>





This patch removes lock_kernel(), unlock_kernel() usage in fbmem.c and replaces it with a mutex

Signed-off-by: Thiago Galesi <thiagogalesi@gmail.com>

---

Ok, 2nd try

The only thing I don't like is the name of fb_ioctl, but I don't have a better idea, so...


---

Index: linux-2.6.23.1/drivers/video/fbmem.c
===================================================================
--- linux-2.6.23.1.orig/drivers/video/fbmem.c
+++ linux-2.6.23.1/drivers/video/fbmem.c
@@ -987,7 +987,7 @@ fb_blank(struct fb_info *info, int blank
 }
 
 static int 
-fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
+__fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 	 unsigned long arg)
 {
 	int fbidx = iminor(inode);
@@ -1085,6 +1085,28 @@ fb_ioctl(struct inode *inode, struct fil
 	}
 }
 
+static int
+fb_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
+	 unsigned long arg)
+{
+	int fbidx = iminor(inode);
+	struct fb_info *info = registered_fb[fbidx];
+	int ret;
+
+	mutex_lock(&info->hwlock);
+	ret = __fb_ioctl(inode, file, cmd, arg);
+	mutex_unlock(&info->hwlock);
+	return ret;
+}
+
+static long
+fb_unlocked_ioctl(struct file *file, unsigned int cmd,
+	 unsigned long arg)
+{
+	struct inode *inode = file->f_path.dentry->d_inode;
+	return fb_ioctl(inode, file, cmd, arg);
+}
+
 #ifdef CONFIG_COMPAT
 struct fb_fix_screeninfo32 {
 	char			id[16];
@@ -1136,7 +1158,7 @@ static int fb_getput_cmap(struct inode *
 	    put_user(compat_ptr(data), &cmap->transp))
 		return -EFAULT;
 
-	err = fb_ioctl(inode, file, cmd, (unsigned long) cmap);
+	err = __fb_ioctl(inode, file, cmd, (unsigned long) cmap);
 
 	if (!err) {
 		if (copy_in_user(&cmap32->start,
@@ -1190,7 +1212,7 @@ static int fb_get_fscreeninfo(struct ino
 
 	old_fs = get_fs();
 	set_fs(KERNEL_DS);
-	err = fb_ioctl(inode, file, cmd, (unsigned long) &fix);
+	err = __fb_ioctl(inode, file, cmd, (unsigned long) &fix);
 	set_fs(old_fs);
 
 	if (!err)
@@ -1208,7 +1230,7 @@ fb_compat_ioctl(struct file *file, unsig
 	struct fb_ops *fb = info->fbops;
 	long ret = -ENOIOCTLCMD;
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 	switch(cmd) {
 	case FBIOGET_VSCREENINFO:
 	case FBIOPUT_VSCREENINFO:
@@ -1217,7 +1239,7 @@ fb_compat_ioctl(struct file *file, unsig
 	case FBIOPUT_CON2FBMAP:
 		arg = (unsigned long) compat_ptr(arg);
 	case FBIOBLANK:
-		ret = fb_ioctl(inode, file, cmd, arg);
+		ret = __fb_ioctl(inode, file, cmd, arg);
 		break;
 
 	case FBIOGET_FSCREENINFO:
@@ -1234,7 +1256,7 @@ fb_compat_ioctl(struct file *file, unsig
 			ret = fb->fb_compat_ioctl(info, cmd, arg);
 		break;
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return ret;
 }
 #endif
@@ -1256,13 +1278,13 @@ fb_mmap(struct file *file, struct vm_are
 		return -ENODEV;
 	if (fb->fb_mmap) {
 		int res;
-		lock_kernel();
+		mutex_lock(&info->hwlock);
 		res = fb->fb_mmap(info, vma);
-		unlock_kernel();
+		mutex_unlock(&info->hwlock);
 		return res;
 	}
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 
 	/* frame buffer memory */
 	start = info->fix.smem_start;
@@ -1271,13 +1293,13 @@ fb_mmap(struct file *file, struct vm_are
 		/* memory mapped io */
 		off -= len;
 		if (info->var.accel_flags) {
-			unlock_kernel();
+			mutex_unlock(&info->hwlock);
 			return -EINVAL;
 		}
 		start = info->fix.mmio_start;
 		len = PAGE_ALIGN((start & ~PAGE_MASK) + info->fix.mmio_len);
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	start &= PAGE_MASK;
 	if ((vma->vm_end - vma->vm_start + off) > len)
 		return -EINVAL;
@@ -1323,25 +1345,25 @@ fb_release(struct inode *inode, struct f
 {
 	struct fb_info * const info = file->private_data;
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 	if (info->fbops->fb_release)
 		info->fbops->fb_release(info,1);
 	module_put(info->fbops->owner);
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return 0;
 }
 
 static const struct file_operations fb_fops = {
-	.owner =	THIS_MODULE,
-	.read =		fb_read,
-	.write =	fb_write,
-	.ioctl =	fb_ioctl,
+	.owner =		THIS_MODULE,
+	.read =			fb_read,
+	.write =		fb_write,
+	.unlocked_ioctl =	fb_unlocked_ioctl,
 #ifdef CONFIG_COMPAT
-	.compat_ioctl = fb_compat_ioctl,
+	.compat_ioctl = 	fb_compat_ioctl,
 #endif
-	.mmap =		fb_mmap,
-	.open =		fb_open,
-	.release =	fb_release,
+	.mmap =			fb_mmap,
+	.open =			fb_open,
+	.release =		fb_release,
 #ifdef HAVE_ARCH_FB_UNMAPPED_AREA
 	.get_unmapped_area = get_fb_unmapped_area,
 #endif
@@ -1413,6 +1435,8 @@ register_framebuffer(struct fb_info *fb_
 
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+
+	mutex_init(&fb_info->hwlock);
 	return 0;
 }
 
@@ -1464,6 +1488,7 @@ unregister_framebuffer(struct fb_info *f
 	device_destroy(fb_class, MKDEV(FB_MAJOR, i));
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event);
+	mutex_destroy(&fb_info.hwlock);
 done:
 	return ret;
 }
Index: linux-2.6.23.1/include/linux/fb.h
===================================================================
--- linux-2.6.23.1.orig/include/linux/fb.h
+++ linux-2.6.23.1/include/linux/fb.h
@@ -802,6 +802,7 @@ struct fb_info {
 	struct list_head modelist;      /* mode list */
 	struct fb_videomode *mode;	/* current mode */
 
+	struct mutex hwlock;		/* mutex for protecting hw ops */
 #ifdef CONFIG_FB_BACKLIGHT
 	/* assigned backlight device */
 	/* set before framebuffer registration, 







  reply	other threads:[~2008-04-16 23:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-13 14:50 [PATCH] fb: Remove use of lock_kernel / unlock_kernel in fbmem Thiago Galesi
2008-04-16  5:17 ` Andrew Morton
2008-04-16  5:17   ` [Linux-fbdev-devel] " Andrew Morton
2008-04-16 23:00   ` Thiago Galesi [this message]
2008-04-16 23:00     ` Thiago Galesi
2008-04-18 17:41     ` Andres Salomon
2008-04-18 17:41       ` [Linux-fbdev-devel] " Andres Salomon
2008-04-19  0:54   ` Thiago Galesi
2008-04-19  0:54     ` [Linux-fbdev-devel] " Thiago Galesi
2008-04-19  1:45     ` Andres Salomon
2008-04-19  1:45       ` [Linux-fbdev-devel] " Andres Salomon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200804162000.59587.thiagogalesi@gmail.com \
    --to=thiagogalesi@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fbdev-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.