[PATCH] nvram: Drop the bkl from non-generic nvram_llseek()

[PATCH] nvram: Drop the bkl from non-generic nvram_llseek()

[Frederic Weisbecker]

Drop the bkl from nvram_llseek() as it obviously protects nothing.
The file offset is safe in essence.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: John Kacur <jkacur@redhat.com>
Cc: Sven-Thorsten Dietrich <sven@thebigcorporation.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Alessio Igor Bogani <abogani@texware.it>
Cc: Greg KH <gregkh@suse.de>
---
 drivers/char/nvram.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index b2a7eaf..d1e3987 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -214,7 +214,6 @@ void nvram_set_checksum(void)
 
 static loff_t nvram_llseek(struct file *file, loff_t offset, int origin)
 {
-	lock_kernel();
 	switch (origin) {
 	case 0:
 		/* nothing to do */
@@ -226,7 +225,7 @@ static loff_t nvram_llseek(struct file *file, loff_t offset, int origin)
 		offset += NVRAM_BYTES;
 		break;
 	}
-	unlock_kernel();
+
 	return (offset >= 0) ? (file->f_pos = offset) : -EINVAL;
 }
 
-- 
1.6.2.3

[tip:bkl/drivers] nvram: Drop the bkl from non-generic nvram_llseek()

[tip-bot for Frederic Weisbecker]

Commit-ID:  9e8ab74ddaa591575f599248080a1f0d917a56ee
Gitweb:     http://git.kernel.org/tip/9e8ab74ddaa591575f599248080a1f0d917a56ee
Author:     Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Fri, 9 Oct 2009 21:27:06 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 14 Oct 2009 17:36:50 +0200

nvram: Drop the bkl from non-generic nvram_llseek()

Drop the bkl from nvram_llseek() as it obviously protects nothing.
The file offset is safe in essence.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <1255116426-7270-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/char/nvram.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 88cee40..2100a8f 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -38,7 +38,6 @@
 #define NVRAM_VERSION	"1.3"
 
 #include <linux/module.h>
-#include <linux/smp_lock.h>
 #include <linux/nvram.h>
 
 #define PC		1
@@ -214,7 +213,6 @@ void nvram_set_checksum(void)
 
 static loff_t nvram_llseek(struct file *file, loff_t offset, int origin)
 {
-	lock_kernel();
 	switch (origin) {
 	case 0:
 		/* nothing to do */
@@ -226,7 +224,7 @@ static loff_t nvram_llseek(struct file *file, loff_t offset, int origin)
 		offset += NVRAM_BYTES;
 		break;
 	}
-	unlock_kernel();
+
 	return (offset >= 0) ? (file->f_pos = offset) : -EINVAL;
 }
 

[tip:bkl/drivers] nvram: Drop the BKL from nvram_open()

[tip-bot for Ingo Molnar]

Commit-ID:  83cb16727085b18191f45eb0ede6bf1f97d67a7a
Gitweb:     http://git.kernel.org/tip/83cb16727085b18191f45eb0ede6bf1f97d67a7a
Author:     Ingo Molnar <mingo@elte.hu>
AuthorDate: Wed, 14 Oct 2009 17:48:38 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 14 Oct 2009 17:54:03 +0200

nvram: Drop the BKL from nvram_open()

It's safe to remove the BKL from nvram_open(): there's no open()
versus read() races: nvram_init() is very simple and race-free,
it registers the device then puts it into /proc - there's no
state init to race with.

Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
LKML-Reference: <1255116426-7270-1-git-send-email-fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/char/nvram.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 2100a8f..7cf4518 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -329,14 +329,12 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
 
 static int nvram_open(struct inode *inode, struct file *file)
 {
-	lock_kernel();
 	spin_lock(&nvram_state_lock);
 
 	if ((nvram_open_cnt && (file->f_flags & O_EXCL)) ||
 	    (nvram_open_mode & NVRAM_EXCL) ||
 	    ((file->f_mode & FMODE_WRITE) && (nvram_open_mode & NVRAM_WRITE))) {
 		spin_unlock(&nvram_state_lock);
-		unlock_kernel();
 		return -EBUSY;
 	}
 
@@ -347,7 +345,6 @@ static int nvram_open(struct inode *inode, struct file *file)
 	nvram_open_cnt++;
 
 	spin_unlock(&nvram_state_lock);
-	unlock_kernel();
 
 	return 0;
 }

Re: [tip:bkl/drivers] nvram: Drop the BKL from nvram_open()

[John Kacur]

On Wed, 14 Oct 2009, tip-bot for Ingo Molnar wrote:

> Commit-ID:  83cb16727085b18191f45eb0ede6bf1f97d67a7a
> Gitweb:     http://git.kernel.org/tip/83cb16727085b18191f45eb0ede6bf1f97d67a7a
> Author:     Ingo Molnar <mingo@elte.hu>
> AuthorDate: Wed, 14 Oct 2009 17:48:38 +0200
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Wed, 14 Oct 2009 17:54:03 +0200
> 
> nvram: Drop the BKL from nvram_open()
> 
> It's safe to remove the BKL from nvram_open(): there's no open()
> versus read() races: nvram_init() is very simple and race-free,
> it registers the device then puts it into /proc - there's no
> state init to race with.
> 
> Cc: Wim Van Sebroeck <wim@iguana.be>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> LKML-Reference: <1255116426-7270-1-git-send-email-fweisbec@gmail.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  drivers/char/nvram.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
> index 2100a8f..7cf4518 100644
> --- a/drivers/char/nvram.c
> +++ b/drivers/char/nvram.c
> @@ -329,14 +329,12 @@ static int nvram_ioctl(struct inode *inode, struct file *file,
>  
>  static int nvram_open(struct inode *inode, struct file *file)
>  {
> -	lock_kernel();
>  	spin_lock(&nvram_state_lock);
>  
>  	if ((nvram_open_cnt && (file->f_flags & O_EXCL)) ||
>  	    (nvram_open_mode & NVRAM_EXCL) ||
>  	    ((file->f_mode & FMODE_WRITE) && (nvram_open_mode & NVRAM_WRITE))) {
>  		spin_unlock(&nvram_state_lock);
> -		unlock_kernel();
>  		return -EBUSY;
>  	}
>  
> @@ -347,7 +345,6 @@ static int nvram_open(struct inode *inode, struct file *file)
>  	nvram_open_cnt++;
>  
>  	spin_unlock(&nvram_state_lock);
> -	unlock_kernel();
>  
>  	return 0;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

This patch and a previous one remove the bkl from the open and llseek 
functions. We can also convert to an unlocked_ioctl

>From 8eab1862b12cc851dc2c811261dda7a22d13e83a Mon Sep 17 00:00:00 2001
From: John Kacur <jkacur@redhat.com>
Date: Wed, 21 Oct 2009 21:57:19 +0200
Subject: [PATCH] nvram: Convert nvram_ioctl to unlocked_ioctl

After removing the BKL from open, we can also convert nvram_ioctl to
an unlocked_ioctl. It has it's only spin_lock, and doesn't rely on the BKL

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 drivers/char/nvram.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 7cf4518..6c34279 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -290,8 +290,7 @@ checksum_err:
 	return -EIO;
 }
 
-static int nvram_ioctl(struct inode *inode, struct file *file,
-					unsigned int cmd, unsigned long arg)
+static long nvram_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	int i;
 
@@ -412,13 +411,13 @@ static int nvram_add_proc_fs(void)
 #endif /* CONFIG_PROC_FS */
 
 static const struct file_operations nvram_fops = {
-	.owner		= THIS_MODULE,
-	.llseek		= nvram_llseek,
-	.read		= nvram_read,
-	.write		= nvram_write,
-	.ioctl		= nvram_ioctl,
-	.open		= nvram_open,
-	.release	= nvram_release,
+	.owner			= THIS_MODULE,
+	.llseek			= nvram_llseek,
+	.read			= nvram_read,
+	.write			= nvram_write,
+	.unlocked_ioctl		= nvram_ioctl,
+	.open			= nvram_open,
+	.release		= nvram_release,
 };
 
 static struct miscdevice nvram_dev = {
-- 
1.6.0.6