megaraid /proc under kernel 2.6.2

Hi all,

I was playing around with forward porting the 2.10.1 megaraid driver to use the new megaraid layout as found in kernel 2.6.2. First I wanted to make sure that the default kernel worked as previously, and then to make sure that each of my layered patches kept the same behaviour.

However, the /proc/megaraid directory is no longer populated, and I am not quite sure why.

In megaraid_init() we do a:

mega_proc_dir_entry = proc_mkdir("megaraid", &proc_root);

, which is good, since that throws a /proc/megaraid directory there for us. So far, so good. mega_proc_dir_entry is a static pointer to a struct proc_dir_entry. The module is now loaded, and we get to device scanning, AFAIK, the new "hotplug-aware pci probing and scsi registration" code put in by Christoph means that once the module is loaded, the hotplug code is then responsible for generating the event to activate the driver. This then calls megaraid_probe_one(). Now, this is where it starts to get a little tricky. There are five different error conditions that do not do a printk to notify of failure, I will send a patch through to address this later, however at the moment I don't think that the initialisation is failing, since I can use the drives attached to the board. So, I am assuming that mega_create_proc_entry() is called. The first thing that this does is to call (effectively) proc_mkdir("hba0", mega_proc_dir_entry); The only way that this can fail to create /proc/megaraid/hba0 (as far as I can see) is if mega_proc_dir_entry is null. The only way that it can be null, is if megaraid_probe_one() is called before megaraid_init(), since the megaraid directory is created.

Either that or I have missed something....

But in any case something weird is going on. I have a three megaraid partitions in use, one which is used for swap, one for /boot and one for a device mapper PV, even though my system is running, and using all three devices, the usage count on the megaraid driver is still 0, and I can rmmod the module, although doing so brings up:

tidbit kernel: journal-601, buffer write failed

which is, I assume, reiserfs bitching about the fact that it's backing device has just totally disappeared, and as soon as I try to do anything that wants read access to the disk, that program will freeze, but that is only to be expected. What is not expected is that I can remove a device that is in use...

Does anyone know what is going on? I am investigating further, but I thought that I would throw this out so that people might be able to tell me where to look...

Cheers,
Paul

Hi again, further information to add....

After adding some printk's to figure out what was going on, I am officially confused. I added this line:

printk(KERN_WARNING "megaraid: trying to proc_mkdir index:%d, parent:%p, procdir:%s\n",index, parent, string);

to mega_create_proc_entry() which gives this response when booting the kernel and loading the module:

megaraid: trying to proc_mkdir index:0, parent:00000000, procdir:hba0

Now, the problem is that this means that the following lines:

	controller_proc_dir_entry =
		adapter->controller_proc_dir_entry = proc_mkdir(string, parent);

	if(!controller_proc_dir_entry) {
		printk(KERN_WARNING "\nmegaraid: proc_mkdir failed\n");
		return;
	}

should print a warning and return. Well, it may or may not be returning, but the "proc_mkdir failed" never turns up. But, if you pass NULL into proc_mkdir as the parent, then it will return NULL as a result, or at least it will as far as I can tell.

So, why is that message not turning up?

I have not looked any further into the missing driver references problem, hopefully someone else will know what that problem is :-)

Cheers,
Paul

Hi all,

OK, I now know what the problem is with the /proc entries and the megaraid driver.

It turns out that pci_driver.probe member (in this case megaraid_probe_one()) is called before the module_init() declared function, in this case megaraid_init().

The problem is as follows: megaraid_probe_one calls mega_create_proc_entry() which relies on the fact that megaraid_init() has already been called. Except, that this fact isn't :-)

So, now that I know what the problem is, does anyone know where the bug really lies? I.e;

Should megaraid_init() be called before megaraid_probe_one() is called? That would make this a "kernel bug", for some suitable value of kernel.

or

Should megaraid_probe_one() not rely on megaraid_init() having been previously called? This would then turn it into a driver bug.

If the latter, when I go in to fix this code, can I assume that megaraid_probe_one() will only be called once at a time, i.e. that there is a lock somewhere outside of me that guarantees that I don't have to worry about re-entrancy issues. Please note that the code already assumes this!

Should this be asked on linux-kernel?

Thanks in advance for all replies,
Paul

Hang on. The pci_driver isn't registered magically, it happens by an explicit call. Oh look, here's the problem:

        error = pci_module_init(&megaraid_pci_driver);
        if (error) 
                return error;

#ifdef CONFIG_PROC_FS
        mega_proc_dir_entry = proc_mkdir("megaraid", &proc_root);
        if (!mega_proc_dir_entry) {
                printk(KERN_WARNING
                                "megaraid: failed to create megaraid root\n");
        }

#endif

Just swap these two stanzas and everything will happen in the right order.

"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

I had the same brainwave while at work today... I am planning on reversing those and testing that now... if it works then I will be sending a patch through in about a half hour :-)

For some reason last night I was just assuming that it was hotplug events that were causing megaraid_probe_one() to be called. In reality, I probably should of just gone to sleep earlier ;-)

Thanks for the input,
Paul

Maybe I can address the issue another way :)

We are moving away from procfs... so may I suggest taking this opportunity to consider what procfs outputs, and a better way to provide that to userspace. Perhaps this information is already duplicated in sysfs...

Jeff

I think you miss the point... I was looking at _decreasing_ the amount of work I had to do! :-)

Anyway, this sysfs stuff is something for me to look at, but at the moment there are no attributes being exported via sysfs, I am currently reading up on the lwn.net articles about it... I will try to submit a patch to add some of this stuff later this week.

However, for at least edification purposes, I would still like to know why the proc directory is not working. It appears to me that a static global variable is not being read somehow?!? I sent a second e-mail through about that though.

Thanks for the input,
Paul

megaraid doesn't set the moduler owner for the host_template. Fix is below.

I don't have the lsightest idea about the procfs issue, though (and no megaraid card neraby to test)

===== drivers/scsi/megaraid.c 1.59 vs edited =====

@@ -4614,6 +4614,7 @@
 }
 
 static struct scsi_host_template megaraid_template = {
+	.module				= THIS_MODULE,
 	.name				= "MegaRAID",
 	.proc_name			= "megaraid",
 	.info				= megaraid_info,

This patch does indeed fix the problem, thank you!

I still have no idea what is happening here, but it is tax time so I don't really have the time to look tonight ;-) I will try to look further at it tomorrow...

Cheers,
Paul