megaraid /proc under kernel 2.6.2

megaraid /proc under kernel 2.6.2

[Paul Wagland]

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

Re: megaraid /proc under kernel 2.6.2

[Paul Wagland]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

Hi again, further information to add....

On Sun, 2004-02-15 at 23:41, Paul Wagland wrote:
> 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.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: megaraid /proc under kernel 2.6.2

[Jeff Garzik]

Paul Wagland wrote:
> 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.


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

Re: megaraid /proc under kernel 2.6.2

[Paul Wagland]

On Mon, 2004-02-16 at 02:56, Jeff Garzik wrote:
> Paul Wagland wrote:
> > 
> > However, the /proc/megaraid directory is no longer populated, and I am
> > not quite sure why.
> 
> 
> 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...

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

Re: megaraid /proc under kernel 2.6.2

[Christoph Hellwig]

On Sun, Feb 15, 2004 at 11:41:31PM +0100, Paul Wagland wrote:
> 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...

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 =====
--- 1.59/drivers/scsi/megaraid.c	Fri Jan 23 06:37:03 2004
+++ edited/drivers/scsi/megaraid.c	Sun Feb 15 00:52:51 2004
@@ -4614,6 +4614,7 @@
 }
 
 static struct scsi_host_template megaraid_template = {
+	.module				= THIS_MODULE,
 	.name				= "MegaRAID",
 	.proc_name			= "megaraid",
 	.info				= megaraid_info,

Re: megaraid /proc under kernel 2.6.2

[Paul Wagland]

On Mon, 2004-02-16 at 10:59, Christoph Hellwig wrote:
> On Sun, Feb 15, 2004 at 11:41:31PM +0100, Paul Wagland wrote:
> > 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:

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

This patch does indeed fix the problem, thank you!

> ===== drivers/scsi/megaraid.c 1.59 vs edited =====
> --- 1.59/drivers/scsi/megaraid.c	Fri Jan 23 06:37:03 2004
> +++ edited/drivers/scsi/megaraid.c	Sun Feb 15 00:52:51 2004
> @@ -4614,6 +4614,7 @@
>  }
>  
>  static struct scsi_host_template megaraid_template = {
> +	.module				= THIS_MODULE,
>  	.name				= "MegaRAID",
>  	.proc_name			= "megaraid",
>  	.info				= megaraid_info,
> -


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

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

Re: *solved* megaraid /proc under kernel 2.6.2

[Paul Wagland]

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

Re: *solved* megaraid /proc under kernel 2.6.2

[Matthew Wilcox]

On Tue, Feb 17, 2004 at 12:04:33AM +0100, Paul Wagland wrote:
> 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().

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

Re: *solved* megaraid /proc under kernel 2.6.2

[Paul Wagland]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]


On Feb 17, 2004, at 17:57, Matthew Wilcox wrote:

> On Tue, Feb 17, 2004 at 12:04:33AM +0100, Paul Wagland wrote:
>> 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().
>
> Hang on.  The pci_driver isn't registered magically, it happens by an
> explicit call.  Oh look, here's the problem:

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]