Re: [PATCH RFC] [PATCH] drivers/scsi/ch.c: Remove BKL in ch_open

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

On Sun, 11 Oct 2009 15:45:16 +0200 (CEST)
John Kacur <jkacur@redhat.com> wrote:

[linux-scsi cc'd]

> Locking in ch_open is covered by the spin_lock, it serializes the calls
> to idr_find and scsi_device_get. The BKL appears redundant to me here.

I'm not so sure. In fact there are some quite umm interesting questions
about this code, and some of them are shared with other modules too.

Consider the following sequence

		CPU1				CPU2
	register_chrdev
		ok
						open device
						takes lock
	scsi_register_driver
		error

	unregister_chrdev
	error
	unload
						??????

We don't allocate any idr entries in open so the paths don't seem to leak
and the module itself looks correct. Looking at the bigger picture
however I am not sure what the module loader is trying to do in
kernel/module.c with the code at

     if (ret < 0) {
                /* Init routine failed: abort.  Try to protect us from
                   buggy refcounters. */
                mod->state = MODULE_STATE_GOING;
                synchronize_sched();
                module_put(mod);
                blocking_notifier_call_chain(&module_notify_list,

but it doesn't seem to be sufficient for what may go on ?

Rusty ?