Messages in this thread | | | Date | Wed, 21 May 2008 12:22:46 -0400 | From | "Mike Frysinger" <> | Subject | Re: [PATCH 1/3, RFC] misc char dev BKL pushdown |
| |
On Tue, May 20, 2008 at 7:25 PM, Jonathan Corbet wrote: > Mike Frysinger wrote: >> please drop the coreb.c changes from your patch > > At a minimum, I would hope such a request would say something like "I've > looked at the driver's locking and am convinced that the BKL is not > needed." Have you done that? There is a certain leap of faith involved > in removing that protection from a driver. > > I decided to take a quick look... > > - You use spin_lock_irq(&coreb_lock) in a number of places, but you do > not take the lock in the interrupt handler. You also do not take the > lock in coreb_write() or coreb_read(), so those can race with the > interrupt handler, with ioctl(), and with each other.
the lock is to protect one thing: coreb_status. we lock around any access to it, so it not being grabbed in the irq handler or any other function where coreb_status is not utilized is irrelevant. that means the BKL is not needed in the driver.
the rest of your comments are more or less on target, but again irrelevant to the topic of the BKL. i'll keep them in mind when i rewrite the driver, thanks. -mike
| |