2009-12-09 17:44:27

by Frederic Weisbecker

[permalink] [raw]
Subject: [PATCH] nvram: Fix missing smp_lock.h in nvram

2009/12/6, Thomas Gleixner <[email protected]>:
> Linus,
>
> Please pull the latest bkl-drivers-for-linus git tree from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git
> bkl-drivers-for-linus
>
> Thanks,
>
> tglx
>
> ------------------>
> Frederic Weisbecker (3):
> mem_class: Drop the bkl from memory_open()
> nvram: Drop the bkl from nvram_llseek()


This breaks the upstream tree. It looks like a patch from Ingo
that removed the bkl in nvram_open() is missing.

Please consider the following patch:

>From 4ce046f911b1cb62b4e952fd599e0d8c3dcd8703 Mon Sep 17 00:00:00 2001
From: Frederic Weisbecker <[email protected]>
Date: Wed, 9 Dec 2009 18:31:53 +0100
Subject: [PATCH] nvram: Fix missing smp_lock.h in nvram

The bkl has been removed from nvram_llseek() and smp_lock.h was
removed because another patch in the same tree zapped the remaining
usage of bkl in the same file. But this patch must have been excluded
later, then we still need the smp_lock.h headers for the bkl use
in nvram_open().

This fixes the following build error:

drivers/char/nvram.c: In function ?nvram_open?:
drivers/char/nvram.c:332: erreur: implicit declaration of function ?lock_kernel?
drivers/char/nvram.c:339: erreur: implicit declaration of function
?unlock_kernel?
make[2]: *** [drivers/char/nvram.o] Erreur 1
make[1]: *** [drivers/char] Erreur 2
make: *** [drivers] Erreur 2

Signed-off-by: Frederic Weisbecker <[email protected]>
---
drivers/char/nvram.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 2100a8f..4008e2c 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -110,6 +110,7 @@
#include <linux/spinlock.h>
#include <linux/io.h>
#include <linux/uaccess.h>
+#include <linux/smp_lock.h>

#include <asm/system.h>

--
1.6.2.3


2009-12-09 22:53:58

by John Kacur

[permalink] [raw]
Subject: Re: [PATCH] nvram: Fix missing smp_lock.h in nvram


On Wed, Dec 9, 2009 at 6:44 PM, Frederic Weisbecker <[email protected]>
wrote:
> 2009/12/6, Thomas Gleixner <[email protected]>:
> > Linus,
> >
> > Please pull the latest bkl-drivers-for-linus git tree from:
> >
> > ? ?git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git
> > bkl-drivers-for-linus
> >
> > Thanks,
> >
> > ? ? ? tglx
> >
> > ------------------>
> > Frederic Weisbecker (3):
> > ? ? ? mem_class: Drop the bkl from memory_open()
> > ? ? ? nvram: Drop the bkl from nvram_llseek()
>
>
> This breaks the upstream tree. It looks like a patch from Ingo
> that removed the bkl in nvram_open() is missing.
>
> Please consider the following patch:
>
> From 4ce046f911b1cb62b4e952fd599e0d8c3dcd8703 Mon Sep 17 00:00:00 2001
> From: Frederic Weisbecker <[email protected]>
> Date: Wed, 9 Dec 2009 18:31:53 +0100
> Subject: [PATCH] nvram: Fix missing smp_lock.h in nvram
>
> The bkl has been removed from nvram_llseek() and smp_lock.h was
> removed because another patch in the same tree zapped the remaining
> usage of bkl in the same file. But this patch must have been excluded
> later, then we still need the smp_lock.h headers for the bkl use
> in nvram_open().
>
> This fixes the following build error:
>
> drivers/char/nvram.c: In function ?nvram_open?:
> drivers/char/nvram.c:332: erreur: implicit declaration of function ?lock_kernel?
> drivers/char/nvram.c:339: erreur: implicit declaration of function
> ?unlock_kernel?
> make[2]: *** [drivers/char/nvram.o] Erreur 1
> make[1]: *** [drivers/char] Erreur 2
> make: *** [drivers] Erreur 2
>
> Signed-off-by: Frederic Weisbecker <[email protected]>
> ---
> ?drivers/char/nvram.c | ? ?1 +
> ?1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
> index 2100a8f..4008e2c 100644
> --- a/drivers/char/nvram.c
> +++ b/drivers/char/nvram.c
> @@ -110,6 +110,7 @@
> ?#include <linux/spinlock.h>
> ?#include <linux/io.h>
> ?#include <linux/uaccess.h>
> +#include <linux/smp_lock.h>
>
> ?#include <asm/system.h>
>
> --
> 1.6.2.3
> --

Ingo's patch was just probably just misplaced, I'd rather that we
resubmit it than put back smp_lock.h

Plus I have a follow-up one that converts nvram_ioctl to unlocked_ioctl.

>From 19a26663db8c627a29e4aabb39a1c84ef85e9fb3 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Wed, 14 Oct 2009 17:48:38 +0200
Subject: [PATCH 1/2] 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 <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Thomas Gleixner <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
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 88cee40..740e356 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -331,14 +331,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;
}

@@ -349,7 +347,6 @@ static int nvram_open(struct inode *inode, struct file *file)
nvram_open_cnt++;

spin_unlock(&nvram_state_lock);
- unlock_kernel();

return 0;
}
--
1.6.5.2

>From fa2448aa56537113dcca77eb7b39a0fc72041122 Mon Sep 17 00:00:00 2001
From: John Kacur <[email protected]>
Date: Wed, 21 Oct 2009 21:57:19 +0200
Subject: [PATCH 2/2] 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 own spin_lock, and doesn't rely on the BKL

Signed-off-by: John Kacur <[email protected]>
---
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 740e356..724b9fe 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -292,8 +292,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;

@@ -414,13 +413,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.5.2