2002-10-21 20:25:15

by Cliff White

[permalink] [raw]
Subject: 2.5.44 compile problem: MegaRAID driver


gcc -Wp,-MD,drivers/scsi/.megaraid.o.d -D__KERNEL__ -Iinclude -Wall
-Wstrict-prototypes -Wno-trigraphs -O2 -fomit-frame-pointer
-fno-strict-aliasing -fno-common -pipe -mpreferred-stack-boundary=2
-march=i686 -Iarch/i386/mach-generic -nostdinc -iwithprefix include
-DKBUILD_BASENAME=megaraid -c -o drivers/scsi/megaraid.o
drivers/scsi/megaraid.c
drivers/scsi/megaraid.c: In function `mega_cmd_done':
drivers/scsi/megaraid.c:1115: warning: passing arg 2 of `__constant_memcpy'
makes pointer from integer without a cast
drivers/scsi/megaraid.c:1115: warning: passing arg 2 of `__memcpy' makes
pointer from integer without a cast
drivers/scsi/megaraid.c: In function `mega_reorder_hosts':
drivers/scsi/megaraid.c:3514: `scsi_hostlist' undeclared (first use in this
function)
drivers/scsi/megaraid.c:3514: (Each undeclared identifier is reported only once
drivers/scsi/megaraid.c:3514: for each function it appears in.)
drivers/scsi/megaraid.c:3514: structure has no member named `next'
drivers/scsi/megaraid.c:3488: warning: `shpnt' might be used uninitialized in
this function
drivers/scsi/megaraid.c: In function `mega_swap_hosts':
drivers/scsi/megaraid.c:3558: structure has no member named `next'
drivers/scsi/megaraid.c:3560: `scsi_hostlist' undeclared (first use in this
function)
drivers/scsi/megaraid.c:3560: structure has no member named `next'
.......repeated bunches
drivers/scsi/megaraid.c:3647: structure has no member named `next'
drivers/scsi/megaraid.c:3648: structure has no member named `next'
drivers/scsi/megaraid.c: In function `megadev_ioctl':
drivers/scsi/megaraid.c:4687: `scsi_hostlist' undeclared (first use in this
function)
drivers/scsi/megaraid.c:4687: structure has no member named `next'
drivers/scsi/megaraid.c:4807: structure has no member named `next'
drivers/scsi/megaraid.c:4546: warning: `shpnt' might be used uninitialized in
this function
make[2]: *** [drivers/scsi/megaraid.o] Error 1
make[1]: *** [drivers/scsi] Error 2
make: *** [drivers] Error 2

cliffw


2002-10-21 21:21:11

by Mike Anderson

[permalink] [raw]
Subject: Re: 2.5.44 compile problem: MegaRAID driver

Cliff,

The scsi host interface change in 2.5.44. The megaraid driver wanted
direct access to this list to reorder it. This cannot be done anymore as
we try get list coherence under control.

I sent mail to [email protected] about this on Saturday. I
have cc'd Matt Domsch on this mail.

You may wish to wait a bit for a response from Matt before applying the
patch.

The patch below removes the reorder as I could not see why reorder the
list. It also replaces some other traverse loops with a direct access to
the host pointer copy.

I have only compile tested this change and do not have a megaraid card.

The maintainer of the driver will need to ok and comment on any side
effects. I will look at the driver some more to see if I understand the
reorder.

-andmike
--
Michael Anderson
[email protected]

megaraid.c | 182 -------------------------------------------------------------
megaraid.h | 2
2 files changed, 2 insertions(+), 182 deletions(-)
------

--- 1.22/drivers/scsi/megaraid.c Tue Oct 8 11:40:48 2002
+++ edited/drivers/scsi/megaraid.c Mon Oct 21 14:10:35 2002
@@ -3217,7 +3217,6 @@
count += mega_findCard (pHostTmpl, PCI_VENDOR_ID_AMI,
PCI_DEVICE_ID_AMI_MEGARAID3, BOARD_QUARTZ);

- mega_reorder_hosts ();

#ifdef CONFIG_PROC_FS
if (count) {
@@ -3483,173 +3482,6 @@
}


-static void mega_reorder_hosts (void)
-{
- struct Scsi_Host *shpnt;
- struct Scsi_Host *shone;
- struct Scsi_Host *shtwo;
- mega_host_config *boot_host;
- int i;
-
- /*
- * Find the (first) host which has it's BIOS enabled
- */
- boot_host = NULL;
- for (i = 0; i < MAX_CONTROLLERS; i++) {
- if (mega_hbas[i].is_bios_enabled) {
- boot_host = mega_hbas[i].hostdata_addr;
- break;
- }
- }
-
- if (boot_host == NULL) {
- printk (KERN_WARNING "megaraid: no BIOS enabled.\n");
- return;
- }
-
- /*
- * Traverse through the list of SCSI hosts for our HBA locations
- */
- shone = shtwo = NULL;
- for (shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next) {
- /* Is it one of ours? */
- for (i = 0; i < MAX_CONTROLLERS; i++) {
- if ((mega_host_config *) shpnt->hostdata ==
- mega_hbas[i].hostdata_addr) {
- /* Does this one has BIOS enabled */
- if (mega_hbas[i].hostdata_addr == boot_host) {
-
- /* Are we first */
- if (shtwo == NULL) /* Yes! */
- return;
- else { /* :-( */
- shone = shpnt;
- }
- } else {
- if (!shtwo) {
- /* were we here before? xchng first */
- shtwo = shpnt;
- }
- }
- break;
- }
- }
- /*
- * Have we got the boot host and one which does not have the bios
- * enabled.
- */
- if (shone && shtwo)
- break;
- }
- if (shone && shtwo) {
- mega_swap_hosts (shone, shtwo);
- }
-
- return;
-}
-
-static void mega_swap_hosts (struct Scsi_Host *shone, struct Scsi_Host *shtwo)
-{
- struct Scsi_Host *prevtoshtwo;
- struct Scsi_Host *prevtoshone;
- struct Scsi_Host *save = NULL;;
-
- /* Are these two nodes adjacent */
- if (shtwo->next == shone) {
-
- if (shtwo == scsi_hostlist && shone->next == NULL) {
-
- /* just two nodes */
- scsi_hostlist = shone;
- shone->next = shtwo;
- shtwo->next = NULL;
- } else if (shtwo == scsi_hostlist) {
- /* first two nodes of the list */
-
- scsi_hostlist = shone;
- shtwo->next = shone->next;
- scsi_hostlist->next = shtwo;
- } else if (shone->next == NULL) {
- /* last two nodes of the list */
-
- prevtoshtwo = scsi_hostlist;
-
- while (prevtoshtwo->next != shtwo)
- prevtoshtwo = prevtoshtwo->next;
-
- prevtoshtwo->next = shone;
- shone->next = shtwo;
- shtwo->next = NULL;
- } else {
- prevtoshtwo = scsi_hostlist;
-
- while (prevtoshtwo->next != shtwo)
- prevtoshtwo = prevtoshtwo->next;
-
- prevtoshtwo->next = shone;
- shtwo->next = shone->next;
- shone->next = shtwo;
- }
-
- } else if (shtwo == scsi_hostlist && shone->next == NULL) {
- /* shtwo at head, shone at tail, not adjacent */
-
- prevtoshone = scsi_hostlist;
-
- while (prevtoshone->next != shone)
- prevtoshone = prevtoshone->next;
-
- scsi_hostlist = shone;
- shone->next = shtwo->next;
- prevtoshone->next = shtwo;
- shtwo->next = NULL;
- } else if (shtwo == scsi_hostlist && shone->next != NULL) {
- /* shtwo at head, shone is not at tail */
-
- prevtoshone = scsi_hostlist;
- while (prevtoshone->next != shone)
- prevtoshone = prevtoshone->next;
-
- scsi_hostlist = shone;
- prevtoshone->next = shtwo;
- save = shtwo->next;
- shtwo->next = shone->next;
- shone->next = save;
- } else if (shone->next == NULL) {
- /* shtwo not at head, shone at tail */
-
- prevtoshtwo = scsi_hostlist;
- prevtoshone = scsi_hostlist;
-
- while (prevtoshtwo->next != shtwo)
- prevtoshtwo = prevtoshtwo->next;
- while (prevtoshone->next != shone)
- prevtoshone = prevtoshone->next;
-
- prevtoshtwo->next = shone;
- shone->next = shtwo->next;
- prevtoshone->next = shtwo;
- shtwo->next = NULL;
-
- } else {
- prevtoshtwo = scsi_hostlist;
- prevtoshone = scsi_hostlist;
- save = NULL;;
-
- while (prevtoshtwo->next != shtwo)
- prevtoshtwo = prevtoshtwo->next;
- while (prevtoshone->next != shone)
- prevtoshone = prevtoshone->next;
-
- prevtoshtwo->next = shone;
- save = shone->next;
- shone->next = shtwo->next;
- prevtoshone->next = shtwo;
- shtwo->next = save;
- }
- return;
-}
-
static inline void mega_freeSgList (mega_host_config * megaCfg)
{
int i;
@@ -4684,12 +4516,7 @@
/*
* Find this host
*/
- for( shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next ) {
- if( shpnt->hostdata == (unsigned long *)megaCtlrs[adapno] ) {
- megacfg = (mega_host_config *)shpnt->hostdata;
- break;
- }
- }
+ shpnt = megaCtlrs[adapno]->host;
if(shpnt == NULL) return -ENODEV;

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
@@ -4804,12 +4631,7 @@
/*
* Find this host
*/
- for( shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next ) {
- if( shpnt->hostdata == (unsigned long *)megaCtlrs[adapno] ) {
- megacfg = (mega_host_config *)shpnt->hostdata;
- break;
- }
- }
+ shpnt = megaCtlrs[adapno]->host;
if(shpnt == NULL) return -ENODEV;

/*
--- 1.9/drivers/scsi/megaraid.h Sun Jul 21 01:55:49 2002
+++ edited/drivers/scsi/megaraid.h Mon Oct 21 13:58:19 2002
@@ -988,8 +988,6 @@
int lock, int intr);

static int mega_is_bios_enabled (mega_host_config *);
-static void mega_reorder_hosts (void);
-static void mega_swap_hosts (struct Scsi_Host *, struct Scsi_Host *);

static void mega_create_proc_entry (int index, struct proc_dir_entry *);
static int mega_support_ext_cdb(mega_host_config *);

2002-10-21 22:04:05

by Matt Domsch

[permalink] [raw]
Subject: RE: 2.5.44 compile problem: MegaRAID driver

> The scsi host interface change in 2.5.44. The megaraid driver wanted
> direct access to this list to reorder it. This cannot be done
> anymore as we try get list coherence under control.
> The patch below removes the reorder as I could not see why reorder the
> list. It also replaces some other traverse loops with a
> direct access to the host pointer copy.

The host reordering is to solve the same problem that EDD helps us solve now
- it makes sure that in systems with megaraid adapters that also have BIOS
enabled (thus have the bootable logical drive on that card) that it shows up
first (/dev/sda) on the host list instead of later. Fortunately, newer
megaraid cards support EDD properly, some older ones don't :-( All that
code is a figment of the need to know where the boot disk is, and to project
it first in the order.

For now, yes, the right thing is to delete those routines and get the
megaraid driver functional again sans adapter reordering. In megaraid2 for
2.5.x we'll need to figure out a better way to handle this - maybe we won't
have to if EDD works as advertised. :-)

> The maintainer of the driver will need to ok and comment on any side
> effects. I will look at the driver some more to see if I
> understand the reorder.

[email protected] has the right people subscribed (subscribe at
http://lists.us.dell.com) - it's really LSI's driver and I want them to be
the folks maintaining it. I've just helped out now and again... LSI hasn't
had time to look at 2.5.x at all.

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer, Architect
Dell Linux Solutions http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

2002-10-21 22:18:17

by Doug Ledford

[permalink] [raw]
Subject: Re: 2.5.44 compile problem: MegaRAID driver

On Mon, Oct 21, 2002 at 11:31:47PM +0100, Alan Cox wrote:
> On Mon, 2002-10-21 at 23:10, [email protected] wrote:
> > The host reordering is to solve the same problem that EDD helps us solve now
> > - it makes sure that in systems with megaraid adapters that also have BIOS
> > enabled (thus have the bootable logical drive on that card) that it shows up
>
> You can fix the ordering up still if you want within cards of a given
> driver type. i2o does this to get its bios boot volume first. Just do it
> by probing your devices, then registering them in the order you want,
> not by mashing the list

This is, umm, a non-elegant way of handling things once you switch your
driver to the new PCI driver probe model :-(

Of course, I'm personally of the opinion that people need to quite
thinking in terms of host order anyway and let things like mount by volume
solve this issue anyway. It's cleaner, it works regardless of the driver,
and it puts the burden of finding the right root partition in user space
where it's easier to fix up should things change, etc. Just my opinion.

--
Doug Ledford <[email protected]> 919-754-3700 x44233
Red Hat, Inc.
1801 Varsity Dr.
Raleigh, NC 27606

2002-10-21 22:09:54

by Alan

[permalink] [raw]
Subject: RE: 2.5.44 compile problem: MegaRAID driver

On Mon, 2002-10-21 at 23:10, [email protected] wrote:
> The host reordering is to solve the same problem that EDD helps us solve now
> - it makes sure that in systems with megaraid adapters that also have BIOS
> enabled (thus have the bootable logical drive on that card) that it shows up

You can fix the ordering up still if you want within cards of a given
driver type. i2o does this to get its bios boot volume first. Just do it
by probing your devices, then registering them in the order you want,
not by mashing the list

2002-10-21 22:23:40

by Matt Domsch

[permalink] [raw]
Subject: Re: 2.5.44 compile problem: MegaRAID driver

> Of course, I'm personally of the opinion that people need to quite
> thinking in terms of host order anyway and let things like mount by volume
> solve this issue anyway. It's cleaner, it works regardless of the driver,
> and it puts the burden of finding the right root partition in user space
> where it's easier to fix up should things change, etc.

EDD lets the OS Installer decide on which unkissed disk it should first
put the OS loader and root partition. On kissed disks
mount-by-{fs,partitiontable}-label/uuid/whatever is great I agree. It's
the unkissed disks that really mess you up.

Thanks,
Matt

--
Matt Domsch
Sr. Software Engineer, Lead Engineer, Architect
Dell Linux Solutions http://www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com


2002-10-21 22:27:20

by Alan Cox

[permalink] [raw]
Subject: Re: 2.5.44 compile problem: MegaRAID driver

> This is, umm, a non-elegant way of handling things once you switch your
> driver to the new PCI driver probe model :-(

Someone has to fix all the problems with the new probe model first. Like
the fact we don't have any meaningful address space locking or
sane refcounting for the scsi objects. Until that happens its not a relevant
discussion IMHO

When you can answer "what lock prevents my address space being reissued
before all memory/I/O has provably completely" and "at what point can I
free my scsi structures on a hot unplug safely for all situations" then
its worth discussion

2002-10-21 22:26:55

by Cliff White

[permalink] [raw]
Subject: Re: 2.5.44 compile problem: MegaRAID driver

> Cliff,
>
> The scsi host interface change in 2.5.44. The megaraid driver wanted
> direct access to this list to reorder it. This cannot be done anymore as
> we try get list coherence under control.
>
> I sent mail to [email protected] about this on Saturday. I
> have cc'd Matt Domsch on this mail.
>
> You may wish to wait a bit for a response from Matt before applying the
> patch.
>
> The patch below removes the reorder as I could not see why reorder the
> list. It also replaces some other traverse loops with a direct access to
> the host pointer copy.
>
> I have only compile tested this change and do not have a megaraid card.
>
> The maintainer of the driver will need to ok and comment on any side
> effects. I will look at the driver some more to see if I understand the
> reorder.
>
> -andmike

The patch compiles clean, the system boots and i've done fdisk and mke2fs
without
any kernel errors.
So far so good. Am running the DBT1 test, will report any errors. Thanks much,
mikeand!
cliffw

> --
> Michael Anderson
> [email protected]
>
> megaraid.c | 182 -------------------------------------------------------------
> megaraid.h | 2
> 2 files changed, 2 insertions(+), 182 deletions(-)
> ------
>
> --- 1.22/drivers/scsi/megaraid.c Tue Oct 8 11:40:48 2002
> +++ edited/drivers/scsi/megaraid.c Mon Oct 21 14:10:35 2002
> @@ -3217,7 +3217,6 @@
> count += mega_findCard (pHostTmpl, PCI_VENDOR_ID_AMI,
> PCI_DEVICE_ID_AMI_MEGARAID3, BOARD_QUARTZ);
>
> - mega_reorder_hosts ();
>
> #ifdef CONFIG_PROC_FS
> if (count) {
> @@ -3483,173 +3482,6 @@
> }
>
>
> -static void mega_reorder_hosts (void)
> -{
> - struct Scsi_Host *shpnt;
> - struct Scsi_Host *shone;
> - struct Scsi_Host *shtwo;
> - mega_host_config *boot_host;
> - int i;
> -
> - /*
> - * Find the (first) host which has it's BIOS enabled
> - */
> - boot_host = NULL;
> - for (i = 0; i < MAX_CONTROLLERS; i++) {
> - if (mega_hbas[i].is_bios_enabled) {
> - boot_host = mega_hbas[i].hostdata_addr;
> - break;
> - }
> - }
> -
> - if (boot_host == NULL) {
> - printk (KERN_WARNING "megaraid: no BIOS enabled.\n");
> - return;
> - }
> -
> - /*
> - * Traverse through the list of SCSI hosts for our HBA locations
> - */
> - shone = shtwo = NULL;
> - for (shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next) {
> - /* Is it one of ours? */
> - for (i = 0; i < MAX_CONTROLLERS; i++) {
> - if ((mega_host_config *) shpnt->hostdata ==
> - mega_hbas[i].hostdata_addr) {
> - /* Does this one has BIOS enabled */
> - if (mega_hbas[i].hostdata_addr == boot_host) {
> -
> - /* Are we first */
> - if (shtwo == NULL) /* Yes! */
> - return;
> - else { /* :-( */
> - shone = shpnt;
> - }
> - } else {
> - if (!shtwo) {
> - /* were we here before? xchng first */
> - shtwo = shpnt;
> - }
> - }
> - break;
> - }
> - }
> - /*
> - * Have we got the boot host and one which does not have the bios
> - * enabled.
> - */
> - if (shone && shtwo)
> - break;
> - }
> - if (shone && shtwo) {
> - mega_swap_hosts (shone, shtwo);
> - }
> -
> - return;
> -}
> -
> -static void mega_swap_hosts (struct Scsi_Host *shone, struct Scsi_Host *shtwo)
> -{
> - struct Scsi_Host *prevtoshtwo;
> - struct Scsi_Host *prevtoshone;
> - struct Scsi_Host *save = NULL;;
> -
> - /* Are these two nodes adjacent */
> - if (shtwo->next == shone) {
> -
> - if (shtwo == scsi_hostlist && shone->next == NULL) {
> -
> - /* just two nodes */
> - scsi_hostlist = shone;
> - shone->next = shtwo;
> - shtwo->next = NULL;
> - } else if (shtwo == scsi_hostlist) {
> - /* first two nodes of the list */
> -
> - scsi_hostlist = shone;
> - shtwo->next = shone->next;
> - scsi_hostlist->next = shtwo;
> - } else if (shone->next == NULL) {
> - /* last two nodes of the list */
> -
> - prevtoshtwo = scsi_hostlist;
> -
> - while (prevtoshtwo->next != shtwo)
> - prevtoshtwo = prevtoshtwo->next;
> -
> - prevtoshtwo->next = shone;
> - shone->next = shtwo;
> - shtwo->next = NULL;
> - } else {
> - prevtoshtwo = scsi_hostlist;
> -
> - while (prevtoshtwo->next != shtwo)
> - prevtoshtwo = prevtoshtwo->next;
> -
> - prevtoshtwo->next = shone;
> - shtwo->next = shone->next;
> - shone->next = shtwo;
> - }
> -
> - } else if (shtwo == scsi_hostlist && shone->next == NULL) {
> - /* shtwo at head, shone at tail, not adjacent */
> -
> - prevtoshone = scsi_hostlist;
> -
> - while (prevtoshone->next != shone)
> - prevtoshone = prevtoshone->next;
> -
> - scsi_hostlist = shone;
> - shone->next = shtwo->next;
> - prevtoshone->next = shtwo;
> - shtwo->next = NULL;
> - } else if (shtwo == scsi_hostlist && shone->next != NULL) {
> - /* shtwo at head, shone is not at tail */
> -
> - prevtoshone = scsi_hostlist;
> - while (prevtoshone->next != shone)
> - prevtoshone = prevtoshone->next;
> -
> - scsi_hostlist = shone;
> - prevtoshone->next = shtwo;
> - save = shtwo->next;
> - shtwo->next = shone->next;
> - shone->next = save;
> - } else if (shone->next == NULL) {
> - /* shtwo not at head, shone at tail */
> -
> - prevtoshtwo = scsi_hostlist;
> - prevtoshone = scsi_hostlist;
> -
> - while (prevtoshtwo->next != shtwo)
> - prevtoshtwo = prevtoshtwo->next;
> - while (prevtoshone->next != shone)
> - prevtoshone = prevtoshone->next;
> -
> - prevtoshtwo->next = shone;
> - shone->next = shtwo->next;
> - prevtoshone->next = shtwo;
> - shtwo->next = NULL;
> -
> - } else {
> - prevtoshtwo = scsi_hostlist;
> - prevtoshone = scsi_hostlist;
> - save = NULL;;
> -
> - while (prevtoshtwo->next != shtwo)
> - prevtoshtwo = prevtoshtwo->next;
> - while (prevtoshone->next != shone)
> - prevtoshone = prevtoshone->next;
> -
> - prevtoshtwo->next = shone;
> - save = shone->next;
> - shone->next = shtwo->next;
> - prevtoshone->next = shtwo;
> - shtwo->next = save;
> - }
> - return;
> -}
> -
> static inline void mega_freeSgList (mega_host_config * megaCfg)
> {
> int i;
> @@ -4684,12 +4516,7 @@
> /*
> * Find this host
> */
> - for( shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next ) {
> - if( shpnt->hostdata == (unsigned long *)megaCtlrs[adapno] ) {
> - megacfg = (mega_host_config *)shpnt->hostdata;
> - break;
> - }
> - }
> + shpnt = megaCtlrs[adapno]->host;
> if(shpnt == NULL) return -ENODEV;
>
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
> @@ -4804,12 +4631,7 @@
> /*
> * Find this host
> */
> - for( shpnt = scsi_hostlist; shpnt; shpnt = shpnt->next ) {
> - if( shpnt->hostdata == (unsigned long *)megaCtlrs[adapno] ) {
> - megacfg = (mega_host_config *)shpnt->hostdata;
> - break;
> - }
> - }
> + shpnt = megaCtlrs[adapno]->host;
> if(shpnt == NULL) return -ENODEV;
>
> /*
> --- 1.9/drivers/scsi/megaraid.h Sun Jul 21 01:55:49 2002
> +++ edited/drivers/scsi/megaraid.h Mon Oct 21 13:58:19 2002
> @@ -988,8 +988,6 @@
> int lock, int intr);
>
> static int mega_is_bios_enabled (mega_host_config *);
> -static void mega_reorder_hosts (void);
> -static void mega_swap_hosts (struct Scsi_Host *, struct Scsi_Host *);
>
> static void mega_create_proc_entry (int index, struct proc_dir_entry *);
> static int mega_support_ext_cdb(mega_host_config *);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>