Re: [PATCH] lcd: replace cli()/sti() with spin_lock_irqsave()/spin_unlock_irqrestore()

[Jim Nelson]

Jan Dittmer wrote:
> James Nelson wrote:
>
> > Remove the cli()/sti() calls in drivers/char/lcd.c
>
>
> Why is this cli() there in the first place? ioctl is already
> called under lock_kernel.

First - a warning.  Newbie on the loose, running around, asking for a whack with 
the cluebat.

I had seen other drivers that had cli()/sti() calls in the ioctl functions.  So, 
that is wrong?  Should all of those cli()/sti() calls be removed?

Just to site things properly in my mind, was it always the case that ioctl is 
called under lock_kernel, or is this a relatively recent (2.3+) development?  Some 
of the *really* abandoned drivers haven't been touched in at least that long.

> > Signed-off-by: James Nelson <james4765@xxxxxxxxx>
> >
> > diff -urN --exclude='*~' linux-2.6.10-rc3-mm1-original/drivers/char/lcd.c linux-2.6.10-rc3-mm1/drivers/char/lcd.c
> > --- linux-2.6.10-rc3-mm1-original/drivers/char/lcd.c	2004-12-03 16:53:42.000000000 -0500
> > +++ linux-2.6.10-rc3-mm1/drivers/char/lcd.c	2004-12-17 18:57:10.760197439 -0500
> > @@ -33,6 +33,8 @@
> >
> > #include "lcd.h"
> >
> > +static spinlock_t lcd_lock = SPIN_LOCK_UNLOCKED;
> > +
> > static int lcd_ioctl(struct inode *inode, struct file *file,
> > 		     unsigned int cmd, unsigned long arg);
> >
> > @@ -464,14 +466,13 @@
> > 			}
> >
> > 			printk("Churning and Burning -");
> > -			save_flags(flags);
> > 			for (i = 0; i < FLASH_SIZE; i = i + 128) {
> >
> > 				if (copy_from_user
> > 				    (rom, display.RomImage + i, 128))
> > 					return -EFAULT;
>
>
> The driver is leaking memory, rom is not freed in this case.

Erm.  Didn't notice that.

> Jan
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/