Re: drivers: Push down BKL into various drivers

Re: drivers: Push down BKL into various drivers

[Geert Uytterhoeven]

On Mon, May 24, 2010 at 18:00, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
> index ad730e1..e00a1cc 100644
> --- a/drivers/input/misc/hp_sdc_rtc.c
> +++ b/drivers/input/misc/hp_sdc_rtc.c
> @@ -43,6 +43,7 @@
>  #include <linux/proc_fs.h>
>  #include <linux/poll.h>
>  #include <linux/rtc.h>
> +#include <linux/smp_lock.h>
>  #include <linux/semaphore.h>
>
>  MODULE_AUTHOR("Brian S. Julin <bri@calyx.com>");
> @@ -64,8 +65,8 @@ static DECLARE_WAIT_QUEUE_HEAD(hp_sdc_rtc_wait);
>  static ssize_t hp_sdc_rtc_read(struct file *file, char __user *buf,
>                               size_t count, loff_t *ppos);
>
> -static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file,
> -                           unsigned int cmd, unsigned long arg);
> +static long hp_sdc_rtc_unlocked_ioctl(struct file *file,
> +                                     unsigned int cmd, unsigned long arg);
>
>  static unsigned int hp_sdc_rtc_poll(struct file *file, poll_table *wait);
>
> @@ -512,7 +513,7 @@ static int hp_sdc_rtc_read_proc(char *page, char **start, off_t off,
>         return len;
>  }
>
> -static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file,
> +static int hp_sdc_rtc_ioctl(struct file *file,
>                            unsigned int cmd, unsigned long arg)
>  {
>  #if 1
> @@ -659,14 +660,27 @@ static int hp_sdc_rtc_ioctl(struct inode *inode, struct file *file,
>  #endif
>  }
>
> +static long hp_sdc_rtc_unlocked_ioctl(struct file *file,
> +                                     unsigned int cmd, unsigned long arg)
> +{
> +       int ret;
> +
> +       lock_kernel();
> +       ret = hp_sdc_rtc_ioctl(file, cmd, arg);
> +       unlock_kernel();
> +
> +       return ret;
> +}
> +
> +
>  static const struct file_operations hp_sdc_rtc_fops = {
> -        .owner =       THIS_MODULE,
> -        .llseek =      no_llseek,
> -        .read =                hp_sdc_rtc_read,
> -        .poll =                hp_sdc_rtc_poll,
> -        .ioctl =       hp_sdc_rtc_ioctl,
> -        .open =                hp_sdc_rtc_open,
> -        .fasync =      hp_sdc_rtc_fasync,
> +        .owner =               THIS_MODULE,
> +        .llseek =              no_llseek,
> +        .read =                        hp_sdc_rtc_read,
> +        .poll =                        hp_sdc_rtc_poll,
> +        .unlocked_ioctl =      hp_sdc_rtc_ioctl,
> +        .open =                        hp_sdc_rtc_open,
> +        .fasync =              hp_sdc_rtc_fasync,
>  };
>
>  static struct miscdevice hp_sdc_rtc_dev = {

Something went wrong with this one:

| drivers/input/misc/hp_sdc_rtc.c:681: warning: initialization from
incompatible pointer type
| drivers/input/misc/hp_sdc_rtc.c:665: warning:
‘hp_sdc_rtc_unlocked_ioctl’ defined but not used

(detected while debugging my script to find compiler warning regressions)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

hp_sdc_rtc: fix broken ioctl conversion

[Arnd Bergmann]

Commit 55929332c92 "drivers: Push down BKL into various drivers"
introduced a regression in hp_sdc_rtc, caused by a missing
change of the .unlocked_ioctl pointer to the newly introduced
function.

On Saturday 05 June 2010, Geert Uytterhoeven wrote:
> Something went wrong with this one:
> 
> | drivers/input/misc/hp_sdc_rtc.c:681: warning: initialization from
> incompatible pointer type
> | drivers/input/misc/hp_sdc_rtc.c:665: warning:
> ‘hp_sdc_rtc_unlocked_ioctl’ defined but not used

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
index e00a1cc..c190664 100644
--- a/drivers/input/misc/hp_sdc_rtc.c
+++ b/drivers/input/misc/hp_sdc_rtc.c
@@ -678,7 +678,7 @@ static const struct file_operations hp_sdc_rtc_fops = {
         .llseek =		no_llseek,
         .read =			hp_sdc_rtc_read,
         .poll =			hp_sdc_rtc_poll,
-        .unlocked_ioctl = 	hp_sdc_rtc_ioctl,
+        .unlocked_ioctl =	hp_sdc_rtc_unlocked_ioctl,
         .open =			hp_sdc_rtc_open,
         .fasync =		hp_sdc_rtc_fasync,
 };

Re: hp_sdc_rtc: fix broken ioctl conversion

[Frederic Weisbecker]

On Sat, Jun 05, 2010 at 10:52:21PM +0200, Arnd Bergmann wrote:
> Commit 55929332c92 "drivers: Push down BKL into various drivers"
> introduced a regression in hp_sdc_rtc, caused by a missing
> change of the .unlocked_ioctl pointer to the newly introduced
> function.
> 
> On Saturday 05 June 2010, Geert Uytterhoeven wrote:
> > Something went wrong with this one:
> > 
> > | drivers/input/misc/hp_sdc_rtc.c:681: warning: initialization from
> > incompatible pointer type
> > | drivers/input/misc/hp_sdc_rtc.c:665: warning:
> > ‘hp_sdc_rtc_unlocked_ioctl’ defined but not used
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
> index e00a1cc..c190664 100644
> --- a/drivers/input/misc/hp_sdc_rtc.c
> +++ b/drivers/input/misc/hp_sdc_rtc.c
> @@ -678,7 +678,7 @@ static const struct file_operations hp_sdc_rtc_fops = {
>          .llseek =		no_llseek,
>          .read =			hp_sdc_rtc_read,
>          .poll =			hp_sdc_rtc_poll,
> -        .unlocked_ioctl = 	hp_sdc_rtc_ioctl,
> +        .unlocked_ioctl =	hp_sdc_rtc_unlocked_ioctl,
>          .open =			hp_sdc_rtc_open,
>          .fasync =		hp_sdc_rtc_fasync,
>  };



Queued for .35, thanks!

Re: hp_sdc_rtc: fix broken ioctl conversion

[Geert Uytterhoeven]

On Sat, Jun 5, 2010 at 22:52, Arnd Bergmann <arnd@arndb.de> wrote:
> Commit 55929332c92 "drivers: Push down BKL into various drivers"
> introduced a regression in hp_sdc_rtc, caused by a missing
> change of the .unlocked_ioctl pointer to the newly introduced
> function.
>
> On Saturday 05 June 2010, Geert Uytterhoeven wrote:
>> Something went wrong with this one:
>>
>> | drivers/input/misc/hp_sdc_rtc.c:681: warning: initialization from
>> incompatible pointer type
>> | drivers/input/misc/hp_sdc_rtc.c:665: warning:
>> ‘hp_sdc_rtc_unlocked_ioctl’ defined but not used
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds