Re: [PATCH 1/3, RFC] misc char dev BKL pushdown

[Mike Frysinger]

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
--
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/