fb: Remove use of lock_kernel / unlock_kernel in fbmem

PATCH In review Thiago Galesi 3 participants 6 messages Apr 13, 2008 – Apr 19, 2008 Framebuffer layer

lore

Share & tools

lore

Thiago Galesi

author

2008-04-13 14:50 UTC #0 lore raw

PATCH 1/1 fb: Remove use of lock_kernel / unlock_kernel in fbmem

Hello

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

Signed-off-by Thiago Galesi <[email protected]>

---

drivers/video/fbmem.c +12 -9

Index: linux-2.6.23.1/drivers/video/fbmem.c
===================================================================
@@ -1208,7 +1208,7 @@
 	struct fb_ops *fb = info->fbops;
 	long ret = -ENOIOCTLCMD;
 
-	lock_kernel();
+	mutex_lock(&info->hwlock);
 	switch(cmd) {
 	case FBIOGET_VSCREENINFO:
 	case FBIOPUT_VSCREENINFO:

@@ -1234,7 +1234,7 @@
 			ret = fb->fb_compat_ioctl(info, cmd, arg);
 		break;
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return ret;
 }
 #endif

@@ -1256,13 +1256,13 @@
 		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 +1271,13 @@
 		/* 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,11 +1323,11 @@
 {
 	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;
 }

@@ -1413,6 +1413,8 @@
 
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+
+	mutex_init(&fb_info->hwlock);
 	return 0;
 }

@@ -1464,6 +1466,7 @@
 	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;
 }

include/linux/fb.h +1 -0

Index: linux-2.6.23.1/include/linux/fb.h
===================================================================
@@ -802,6 +802,7 @@
 	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,

Andrew Morton

2008-04-16 05:17 UTC #1 lore raw

Quoted 1 line · On Sun, 13 Apr 2008 11:50:07 -0300 Thiago Galesi <[email protected]> wrote:

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

It isn't that simple, alas.

vfs_ioctl() runs lock_kernel() prior to calling fb_ioctl(), so the lock_kernel()s in fb_compat_ioctl() are actually providing exclusion against fb_ioctl(). Your patch would break that.

A suitable fix might be to do

__fb_ioctl(...)
{
<copy fb_ioctl() into here>
}

fb_ioctl(...)

{
	mutex_lock(&info->hwlock);
	__fb_ioctl(...);
	mutex_unlock(&info->hwlock);
}

and then change fb_compat_ioctl() to call __fb_ioctl(). All the other callers of fb_ioctl() would need to be reviewed - see if they need to take the mutex then call __fb_ioctl(), or they might be OK as they are, calling fb_ioctl().

Then we can switch fb_fops over to

	.ioctl	= NULL,
	.unlocked_ioctl = fb_ioctl,

-------------------------------------------------------------------------
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

Thiago Galesi

author

2008-04-16 23:01 UTC #2 lore raw

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

Signed-off-by Thiago Galesi <[email protected]>

---

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...

---

drivers/video/fbmem.c +46 ~~-21~~

Index: 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;
 }

include/linux/fb.h +1 -0

Index: 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,

Andres Salomon

2008-04-18 17:39 UTC #3 lore raw

Quoted 5 lines · On Wed, 16 Apr 2008 20:00:59 -0300 Thiago Galesi <[email protected]> wrote:

This patch removes lock_kernel(), unlock_kernel() usage in fbmem.c and replaces it with a mutex
Signed-off-by Thiago Galesi <[email protected]>

[...]

drivers/video/fbmem.c register_framebuffer(struct fb_info *fb_ · line 1442

Quoted 9 lines

drivers/video/fbmem.c +2 -0

@@ -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;
 }

Just a minor nit; in general, I'd think you would want to completely initialize the structure (including calling mutex_init) before registering the structure with the rest of the system (in this case, with registered_fb and *_call_notifier_chain). Initializing after registration is just asking for a race conditions.

Signature · 7 lines

Need a kernel or Debian developer?  Contact me, I'm looking for contracts.

-------------------------------------------------------------------------
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

Thiago Galesi

author

2008-04-19 00:54 UTC #4 lore raw

In reply to Andrew Morton

Hello

Quoted 5 lines

Just a minor nit; in general, I'd think you would want to completely initialize the structure (including calling mutex_init) before registering the structure with the rest of the system (in this case, with registered_fb and *_call_notifier_chain). Initializing after registration is just asking for a race conditions.

This is better??

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

Signed-off-by Thiago Galesi <[email protected]>

---

drivers/video/fbmem.c +46 ~~-21~~

Index: linux-2.6.23.1/drivers/video/fbmem.c
===================================================================
@@ -987,7 +987,7 @@
 }
 
 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 @@
 	}
 }
 
+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 @@
 	    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 @@
 
 	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 @@
 	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 @@
 	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 @@
 			ret = fb->fb_compat_ioctl(info, cmd, arg);
 		break;
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return ret;
 }
 #endif

@@ -1256,13 +1278,13 @@
 		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 @@
 		/* 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 @@
 {
 	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

@@ -1377,6 +1399,7 @@
 			break;
 	fb_info->node = i;
 
+	mutex_init(&fb_info->hwlock);
 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), "fb%d", i);
 	if (IS_ERR(fb_info->dev)) {

@@ -1413,6 +1436,7 @@
 
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+
 	return 0;
 }

@@ -1464,6 +1488,7 @@
 	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;
 }

include/linux/fb.h +1 -0

Index: linux-2.6.23.1/include/linux/fb.h
===================================================================
@@ -802,6 +802,7 @@
 	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,

Andres Salomon

2008-04-19 01:42 UTC #5 lore raw

Quoted 10 lines · On Fri, 18 Apr 2008 21:54:43 -0300 Thiago Galesi <[email protected]> wrote:

Hello
Quoted 5 lines
Just a minor nit; in general, I'd think you would want to completely initialize the structure (including calling mutex_init) before registering the structure with the rest of the system (in this case, with registered_fb and *_call_notifier_chain). Initializing after registration is just asking for a race conditions.
This is better??

That does look better, yes. Someone else who is familiar w/ the locking semantics there would have to comment on the rest of it.

Quoted 205 lines

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

Signed-off-by Thiago Galesi <[email protected]>

---

Index: linux-2.6.23.1/drivers/video/fbmem.c
===================================================================

drivers/video/fbmem.c +46 ~~-21~~

@@ -987,7 +987,7 @@
 }
 
 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 @@
 	}
 }
 
+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 @@
 	    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 @@
 
 	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 @@
 	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 @@
 	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 @@
 			ret = fb->fb_compat_ioctl(info, cmd, arg);
 		break;
 	}
-	unlock_kernel();
+	mutex_unlock(&info->hwlock);
 	return ret;
 }
 #endif

@@ -1256,13 +1278,13 @@
 		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 @@
 		/* 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 @@
 {
 	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

@@ -1377,6 +1399,7 @@
 			break;
 	fb_info->node = i;
 
+	mutex_init(&fb_info->hwlock);
 	fb_info->dev = device_create(fb_class, fb_info->device,
 				     MKDEV(FB_MAJOR, i), "fb%d", i);
 	if (IS_ERR(fb_info->dev)) {

@@ -1413,6 +1436,7 @@
 
 	event.info = fb_info;
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
+
 	return 0;
 }

@@ -1464,6 +1488,7 @@
 	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
===================================================================

include/linux/fb.h +1 -0

@@ -802,6 +802,7 @@
 	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,

Signature · 7 lines

Need a kernel or Debian developer?  Contact me, I'm looking for contracts.

-------------------------------------------------------------------------
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