BKL locking came to efirtc via the big BKL push down, but the access
to the efi functions is protected by efi_rtc_lock already.
Remove it.
Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/char/efirtc.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
---
drivers/char/efirtc.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
Index: linux-2.6-tip/drivers/char/efirtc.c
===================================================================
--- linux-2.6-tip.orig/drivers/char/efirtc.c
+++ linux-2.6-tip/drivers/char/efirtc.c
@@ -27,8 +27,6 @@
* - Add module support
*/
-
-#include <linux/smp_lock.h>
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/miscdevice.h>
@@ -174,13 +172,12 @@ static long efi_rtc_ioctl(struct file *f
return -EINVAL;
case RTC_RD_TIME:
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.get_time(&eft, &cap);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
+
if (status != EFI_SUCCESS) {
/* should never happen */
printk(KERN_ERR "efitime: can't read time\n");
@@ -202,13 +199,11 @@ static long efi_rtc_ioctl(struct file *f
convert_to_efi_time(&wtime, &eft);
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.set_time(&eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
return status == EFI_SUCCESS ? 0 : -EINVAL;
@@ -224,7 +219,6 @@ static long efi_rtc_ioctl(struct file *f
convert_to_efi_time(&wtime, &eft);
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
/*
* XXX Fixme:
@@ -235,19 +229,16 @@ static long efi_rtc_ioctl(struct file *f
status = efi.set_wakeup_time((efi_bool_t)enabled, &eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
return status == EFI_SUCCESS ? 0 : -EINVAL;
case RTC_WKALM_RD:
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.get_wakeup_time((efi_bool_t *)&enabled, (efi_bool_t *)&pending, &eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
if (status != EFI_SUCCESS) return -EINVAL;
@@ -277,7 +268,6 @@ static int efi_rtc_open(struct inode *in
* We do accept multiple open files at the same time as we
* synchronize on the per call operation.
*/
- cycle_kernel_lock();
return 0;
}
Commit-ID: a5ee6dc9ebe8fc2640ee3fbf2c340bd853e2fd36
Gitweb: http://git.kernel.org/tip/a5ee6dc9ebe8fc2640ee3fbf2c340bd853e2fd36
Author: Thomas Gleixner <[email protected]>
AuthorDate: Sat, 10 Oct 2009 15:14:03 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 14 Oct 2009 17:36:51 +0200
rtc: Remove BKL from efirtc
BKL locking came to efirtc via the big BKL push down, but the access
to the efi functions is protected by efi_rtc_lock already.
Remove it.
Signed-off-by: Thomas Gleixner <[email protected]>
LKML-Reference: <[email protected]>
---
drivers/char/efirtc.c | 12 +-----------
1 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
index 34d15d5..26a47dc 100644
--- a/drivers/char/efirtc.c
+++ b/drivers/char/efirtc.c
@@ -27,8 +27,6 @@
* - Add module support
*/
-
-#include <linux/smp_lock.h>
#include <linux/types.h>
#include <linux/errno.h>
#include <linux/miscdevice.h>
@@ -174,13 +172,12 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
return -EINVAL;
case RTC_RD_TIME:
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.get_time(&eft, &cap);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
+
if (status != EFI_SUCCESS) {
/* should never happen */
printk(KERN_ERR "efitime: can't read time\n");
@@ -202,13 +199,11 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
convert_to_efi_time(&wtime, &eft);
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.set_time(&eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
return status == EFI_SUCCESS ? 0 : -EINVAL;
@@ -224,7 +219,6 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
convert_to_efi_time(&wtime, &eft);
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
/*
* XXX Fixme:
@@ -235,19 +229,16 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
status = efi.set_wakeup_time((efi_bool_t)enabled, &eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
return status == EFI_SUCCESS ? 0 : -EINVAL;
case RTC_WKALM_RD:
- lock_kernel();
spin_lock_irqsave(&efi_rtc_lock, flags);
status = efi.get_wakeup_time((efi_bool_t *)&enabled, (efi_bool_t *)&pending, &eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
- unlock_kernel();
if (status != EFI_SUCCESS) return -EINVAL;
@@ -277,7 +268,6 @@ static int efi_rtc_open(struct inode *inode, struct file *file)
* We do accept multiple open files at the same time as we
* synchronize on the per call operation.
*/
- cycle_kernel_lock();
return 0;
}
>From e1b7175258b33da3b0564ef04a0b1956f04f0cc7 Mon Sep 17 00:00:00 2001
From: John Kacur <[email protected]>
Date: Wed, 21 Oct 2009 23:10:30 +0200
Subject: [PATCH] rtc: Explicitly set llseek to no_llseek
Now that we've removed the BKL here, lets explicitly set llseek to no_llseek
since the default llseek is not used here.
Signed-off-by: John Kacur <[email protected]>
---
drivers/char/efirtc.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
index 26a47dc..53c524e 100644
--- a/drivers/char/efirtc.c
+++ b/drivers/char/efirtc.c
@@ -285,6 +285,7 @@ static const struct file_operations efi_rtc_fops = {
.unlocked_ioctl = efi_rtc_ioctl,
.open = efi_rtc_open,
.release = efi_rtc_close,
+ .llseek = no_llseek,
};
static struct miscdevice efi_rtc_dev= {
--
1.6.0.6
On Wed, 21 Oct 2009 23:13:26 +0200 (CEST)
John Kacur <[email protected]> wrote:
> >From e1b7175258b33da3b0564ef04a0b1956f04f0cc7 Mon Sep 17 00:00:00 2001
> From: John Kacur <[email protected]>
> Date: Wed, 21 Oct 2009 23:10:30 +0200
> Subject: [PATCH] rtc: Explicitly set llseek to no_llseek
>
> Now that we've removed the BKL here, lets explicitly set llseek to no_llseek
> since the default llseek is not used here.
>
I don't understand.
>
> diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
> index 26a47dc..53c524e 100644
> --- a/drivers/char/efirtc.c
> +++ b/drivers/char/efirtc.c
> @@ -285,6 +285,7 @@ static const struct file_operations efi_rtc_fops = {
> .unlocked_ioctl = efi_rtc_ioctl,
> .open = efi_rtc_open,
> .release = efi_rtc_close,
> + .llseek = no_llseek,
> };
>
> static struct miscdevice efi_rtc_dev= {
What has this change to do with the BKL?
On Tue, 3 Nov 2009, Andrew Morton wrote:
> On Wed, 21 Oct 2009 23:13:26 +0200 (CEST)
> John Kacur <[email protected]> wrote:
>
> > >From e1b7175258b33da3b0564ef04a0b1956f04f0cc7 Mon Sep 17 00:00:00 2001
> > From: John Kacur <[email protected]>
> > Date: Wed, 21 Oct 2009 23:10:30 +0200
> > Subject: [PATCH] rtc: Explicitly set llseek to no_llseek
> >
> > Now that we've removed the BKL here, lets explicitly set llseek to no_llseek
> > since the default llseek is not used here.
> >
>
> I don't understand.
>
> >
> > diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
> > index 26a47dc..53c524e 100644
> > --- a/drivers/char/efirtc.c
> > +++ b/drivers/char/efirtc.c
> > @@ -285,6 +285,7 @@ static const struct file_operations efi_rtc_fops = {
> > .unlocked_ioctl = efi_rtc_ioctl,
> > .open = efi_rtc_open,
> > .release = efi_rtc_close,
> > + .llseek = no_llseek,
> > };
> >
> > static struct miscdevice efi_rtc_dev= {
>
> What has this change to do with the BKL?
The default_llseek function still contains the BKL.
When we are auditing code to see if we can remove the BKL, this is one of
the hidden considerations we need to take into account.
i.e., is there syncronization between code that has the BKL and llseek.
At the same time we remove the BKL it would be a good idea to do indicate
when no llseek function is required, so we don't have to revisit this code
again, when we are trying to determine if we can remove the BKL from
the default_llseek.