i2o_core.c, i2o_scsi.c minor bugfixes

Hello,

after using the i2o modules (i2o_core and i2o_scsi), i found that there seems to be some minor problems in the 2.6.1 kernel version. So i have created a patch, which resolves the following issues:

i2o-cleanup.patch:
------------------
drivers/modules/i2o/i2o_core.c:
- i2oevtd doesn't exit, if module is removed cause of SIGKILL/SIGTERM mismatch.
- cleaup of HRT does result in Segfault, because the length of HRT is not stored in structure.

drivers/modules/i2o/i2o_scsi.c:
- converted dprintk calling style like the one in i2o_core.c
- scsi_unregister() is not called, if module is removed

include/linux/i2o-dev.h:
- fixed a typo in struct _i2o_hrt_entry

i2o-cleanup.patch:
------------------

diff -ur drivers/message/i2o.old/i2o_core.c drivers/message/i2o/i2o_core.c
@@ -502,6 +502,7 @@
 			c->unit = i;
 			c->page_frame = NULL;
 			c->hrt = NULL;
+			c->hrt_len = 0;
 			c->lct = NULL;
 			c->status_block = NULL;
 			sprintf(c->name, "i2o/iop%d", i);

@@ -885,7 +885,7 @@
 	unsigned long flags;
 
 	daemonize("i2oevtd");
-	allow_signal(SIGKILL);
+	allow_signal(SIGTERM);
 
 	evt_running = 1;
 

@@ -1052,7 +1052,7 @@
 	void *tmp;
 
 	daemonize("iop%d_lctd", c->unit);
-	allow_signal(SIGKILL);
+	allow_signal(SIGTERM);
 
 	c->lct_running = 1;
 

@@ -1861,31 +1862,39 @@
 {
 	u32 msg[6];
 	int ret, size = sizeof(i2o_hrt);
+	int loops = 3;	/* we only try 3 times to get the HRT, this should be
+			   more then enough. Worst case should be 2 times.*/
 
 	/* First read just the header to figure out the real size */
 

	do  {

+ /* first we allocate the memory for the HRT */
if (c->hrt == NULL) {
c->hrt=pci_alloc_consistent(c->pdev, size, &c->hrt_phys); if (c->hrt == NULL) {
printk(KERN_CRIT "%s: Hrt Get failed; Out of memory.\n", c->name); return -ENOMEM;
}
+ c->hrt_len = size;
}

msg[0]= SIX_WORD_MSG_SIZE| SGL_OFFSET_4; msg[1]= I2O_CMD_HRT_GET<<24 | HOST_TID<<12 | ADAPTER_TID; msg[3]= 0;
- msg[4]= (0xD0000000 | size); /* Simple transaction */
+ msg[4]= (0xD0000000 | c->hrt_len); /* Simple transaction */
msg[5]= c->hrt_phys; /* Dump it here */

- ret = i2o_post_wait_mem(c, msg, sizeof(msg), 20, c->hrt, NULL, c->hrt_phys, 0, size, 0);
+ ret = i2o_post_wait_mem(c, msg, sizeof(msg), 20, c->hrt, NULL, c->hrt_phys, 0, c->hrt_len, 0);

if(ret == -ETIMEDOUT)
{
- /* The HRT block we used is in limbo somewhere. When the iop wakes up
- we will recover it */
+ /* The HRT block we used is in limbo somewhere. When the
+ iop wakes up we will recover it */
+ /* FIX: shouldn't we cleanup the memory?
+ pci_free_consistent(c->pdev, c->hrt_len, c->hrt, c->hrt_phys);
+ */
c->hrt = NULL;
+ c->hrt_len = 0;
return ret;
}

@@ -1896,13 +1905,20 @@
return ret;
}

- if (c->hrt->num_entries * c->hrt->entry_len << 2 > size) {
- int new_size = c->hrt->num_entries * c->hrt->entry_len << 2;
- pci_free_consistent(c->pdev, size, c->hrt, c->hrt_phys);
- size = new_size;
+ if (c->hrt->num_entries * c->hrt->entry_len << 2 > c->hrt_len) {
+ size = c->hrt->num_entries * c->hrt->entry_len << 2;
+ pci_free_consistent(c->pdev, c->hrt_len, c->hrt, c->hrt_phys);
+ c->hrt_len = 0;
c->hrt = NULL;
}
- } while (c->hrt == NULL);
+ loops --;
+ } while (c->hrt == NULL && loops > 0);
+
+ if(c->hrt == NULL)
+ {
+ printk(KERN_ERR "%s: Unable to get HRT after three tries, giving up\n", c->name);
+ return -1;
+ }

i2o_parse_hrt(c); // just for debugging

@@ -3737,13 +3737,12 @@
printk("Terminating i2o threads..."); stat = kill_proc(evt_pid, SIGTERM, 1); if(!stat) {
- printk("waiting...");
+ printk("waiting...\n");
wait_for_completion(&evt_dead);
}
printk("done.\n");
} i2o_remove_handler(&i2o_core_handler);
- unregister_reboot_notifier(&i2o_reboot_notifier);
}

module_init(i2o_core_init);

@@ -182,7 +182,7 @@
 {
 	u32	adapter_id;
 	u32	parent_tid:12;
-	u32 	tate:4;
+	u32 	state:4;
 	u32	bus_num:8;
 	u32	bus_type:8;
 	union

Also i found that the resource management in i2o_scsi is static, so i tried to make things a little bit dynamic by allocating the mapping tables at module insertion (not included in this e-mail). Because i don't saw a maintainer for those module, i wrote to the kernel mailing list. Can anybody tell me, who i can contact to verify if my changes are of any value? I want to contribute some more code, because on my system the i2o_block doesn't work too. But before, i want to be sure that my code is needed and also is good enough to be included into the kernel.

Thank you very much.

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

I get 100% rejects applying this. Could you resend it as an attachment?

I think that would be Alan Cox <[email protected]>

Hello,

Okey, another try, this time as attachment.

Thank you very much for your help!

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

diff -ur drivers/message/i2o.old/i2o_core.c drivers/message/i2o/i2o_core.c
@@ -502,6 +502,7 @@
 			c->unit = i;
 			c->page_frame = NULL;
 			c->hrt = NULL;
+			c->hrt_len = 0;
 			c->lct = NULL;
 			c->status_block = NULL;
 			sprintf(c->name, "i2o/iop%d", i);

@@ -885,7 +885,7 @@
 	unsigned long flags;
 
 	daemonize("i2oevtd");
-	allow_signal(SIGKILL);
+	allow_signal(SIGTERM);
 
 	evt_running = 1;
 

@@ -1052,7 +1052,7 @@
 	void *tmp;
 
 	daemonize("iop%d_lctd", c->unit);
-	allow_signal(SIGKILL);
+	allow_signal(SIGTERM);
 
 	c->lct_running = 1;
 

@@ -1861,31 +1862,39 @@
 {
 	u32 msg[6];
 	int ret, size = sizeof(i2o_hrt);
+	int loops = 3;	/* we only try 3 times to get the HRT, this should be
+			   more then enough. Worst case should be 2 times.*/
 
 	/* First read just the header to figure out the real size */
 

	do  {

+ /* first we allocate the memory for the HRT */
if (c->hrt == NULL) {
c->hrt=pci_alloc_consistent(c->pdev, size, &c->hrt_phys); if (c->hrt == NULL) {
printk(KERN_CRIT "%s: Hrt Get failed; Out of memory.\n", c->name); return -ENOMEM;
}
+ c->hrt_len = size;
}

msg[0]= SIX_WORD_MSG_SIZE| SGL_OFFSET_4; msg[1]= I2O_CMD_HRT_GET<<24 | HOST_TID<<12 | ADAPTER_TID; msg[3]= 0;
- msg[4]= (0xD0000000 | size); /* Simple transaction */
+ msg[4]= (0xD0000000 | c->hrt_len); /* Simple transaction */
msg[5]= c->hrt_phys; /* Dump it here */

- ret = i2o_post_wait_mem(c, msg, sizeof(msg), 20, c->hrt, NULL, c->hrt_phys, 0, size, 0);
+ ret = i2o_post_wait_mem(c, msg, sizeof(msg), 20, c->hrt, NULL, c->hrt_phys, 0, c->hrt_len, 0);

if(ret == -ETIMEDOUT)
{
- /* The HRT block we used is in limbo somewhere. When the iop wakes up
- we will recover it */
+ /* The HRT block we used is in limbo somewhere. When the
+ iop wakes up we will recover it */
+ /* FIX: shouldn't we cleanup the memory?
+ pci_free_consistent(c->pdev, c->hrt_len, c->hrt, c->hrt_phys);
+ */
c->hrt = NULL;
+ c->hrt_len = 0;
return ret;
}

@@ -1896,13 +1905,20 @@
return ret;
}

- if (c->hrt->num_entries * c->hrt->entry_len << 2 > size) {
- int new_size = c->hrt->num_entries * c->hrt->entry_len << 2;
- pci_free_consistent(c->pdev, size, c->hrt, c->hrt_phys);
- size = new_size;
+ if (c->hrt->num_entries * c->hrt->entry_len << 2 > c->hrt_len) {
+ size = c->hrt->num_entries * c->hrt->entry_len << 2;
+ pci_free_consistent(c->pdev, c->hrt_len, c->hrt, c->hrt_phys);
+ c->hrt_len = 0;
c->hrt = NULL;
}
- } while (c->hrt == NULL);
+ loops --;
+ } while (c->hrt == NULL && loops > 0);
+
+ if(c->hrt == NULL)
+ {
+ printk(KERN_ERR "%s: Unable to get HRT after three tries, giving up\n", c->name);
+ return -1;
+ }

i2o_parse_hrt(c); // just for debugging

@@ -3737,13 +3737,12 @@
printk("Terminating i2o threads..."); stat = kill_proc(evt_pid, SIGTERM, 1); if(!stat) {
- printk("waiting...");
+ printk("waiting...\n");
wait_for_completion(&evt_dead);
}
printk("done.\n");
} i2o_remove_handler(&i2o_core_handler);
- unregister_reboot_notifier(&i2o_reboot_notifier);
}

module_init(i2o_core_init);

@@ -182,7 +182,7 @@
 {
 	u32	adapter_id;
 	u32	parent_tid:12;
-	u32 	tate:4;
+	u32 	state:4;
 	u32	bus_num:8;
 	u32	bus_type:8;
 	union