Skip to contentSkip to conversation

reduce stack in qlogicfc.c

PATCH In review Randy.Dunlap 3 participants 4 messages Mar 12, 2003 SCSI

lore
Share & tools
lore

Randy.Dunlap

author
#0 lore raw

PATCH 1/1 reduce stack in qlogicfc.c

Hi,

This is a start on reducing the stack usage in qlogicfc.c::
isp2x00_make_portdb(). I think that the stack reduction portion of it is fine, but I'm concerned about the function returning early due to kmalloc() failure, without making the port database.

[reduces stack from 0xc38 to 0x34 bytes (P4 UP, gcc 2.96)]

Can anyone suggest way(s) to have the isp2x00_make_portdb() function called over and over again until it gets its job done?
Or does anyone even still use this driver?

Thanks,
~Randy

patch_name: qlogicfc_stack.patch
patch_version: 2003-03-11.21:52:26
author: Randy.Dunlap <[email protected]>
description: reduce stack usage in qlogicfc.c::isp2x00_make_portdb()

		from 0xc38 to 0x34 bytes (P4 UP, gcc 2.96);

product: Linux
product_versions: linux-2564
changelog: change id_name_map from a large array to a kmalloc-ed area;
requires: Linux 2.5.64
maintainer: Erik H. Moe, [email protected]
diffstat: =

 drivers/scsi/qlogicfc.c |   72 +++++++++++++++++++++++++++---------------------
 1 files changed, 41 insertions(+), 31 deletions(-)
drivers/scsi/qlogicfc.c renamed from drivers/scsi/qlogicfc.c%QLOGFC +41 -31
diff -Naur ./drivers/scsi/qlogicfc.c%QLOGFC ./drivers/scsi/qlogicfc.c
@@ -836,14 +836,22 @@
 
 	short param[8];
 	int i, j;
-	struct id_name_map temp[QLOGICFC_MAX_ID + 1];
+	struct id_name_map *map; /* base of array [QLOGICFC_MAX_ID + 1] */
+	struct id_name_map *mapx; /* array entry pointer */
 	struct isp2x00_hostdata *hostdata;
 
-	isp2x00_disable_irqs(host);
-
-	memset(temp, 0, sizeof(temp));
 	hostdata = (struct isp2x00_hostdata *) host->hostdata;
 
+	map = kmalloc((QLOGICFC_MAX_ID + 1) * sizeof(struct id_name_map), GFP_ATOMIC);
+	if (!map) {
+	        printk("qlogicfc%d : error getting memory -- cannot make port database.\n",
+				hostdata->host_id);
+		goto fini;
+	}
+	memset(map, 0, (QLOGICFC_MAX_ID + 1) * sizeof(struct id_name_map));
+
+	isp2x00_disable_irqs(host);
+
 #if ISP2x00_FABRIC
 	for (i = 0x81; i < QLOGICFC_MAX_ID; i++) {
 		param[0] = MBOX_PORT_LOGOUT;

@@ -868,72 +876,74 @@
 	if (param[0] == MBOX_COMMAND_COMPLETE) {
 		hostdata->port_id = ((u_int) param[3]) << 16;
 		hostdata->port_id |= param[2];
-		temp[0].loop_id = param[1];
-		temp[0].wwn = hostdata->wwn;
+		map->loop_id = param[1];
+		map->wwn = hostdata->wwn;
 	}
 	else {
 	        printk("qlogicfc%d : error getting scsi id.\n", hostdata->host_id);
 	}
 
-        for (i = 0; i <=QLOGICFC_MAX_ID; i++)
-                temp[i].loop_id = temp[0].loop_id;
+        for (i = 0, mapx = map; i <= QLOGICFC_MAX_ID; i++, mapx++)
+                mapx->loop_id = map->loop_id;
    
-        for (i = 0, j = 1; i <= QLOGICFC_MAX_LOOP_ID; i++) {
+        for (i = 0, j = 1, mapx = map + 1; i <= QLOGICFC_MAX_LOOP_ID; i++) {
                 param[0] = MBOX_GET_PORT_NAME;
 		param[1] = (i << 8) & 0xff00;
 
 		isp2x00_mbox_command(host, param);
 
 		if (param[0] == MBOX_COMMAND_COMPLETE) {
-			temp[j].loop_id = i;
-			temp[j].wwn = ((u64) (param[2] & 0xff)) << 56;
-			temp[j].wwn |= ((u64) ((param[2] >> 8) & 0xff)) << 48;
-			temp[j].wwn |= ((u64) (param[3] & 0xff)) << 40;
-			temp[j].wwn |= ((u64) ((param[3] >> 8) & 0xff)) << 32;
-			temp[j].wwn |= ((u64) (param[6] & 0xff)) << 24;
-			temp[j].wwn |= ((u64) ((param[6] >> 8) & 0xff)) << 16;
-			temp[j].wwn |= ((u64) (param[7] & 0xff)) << 8;
-			temp[j].wwn |= ((u64) ((param[7] >> 8) & 0xff));
+			mapx->loop_id = i;
+			mapx->wwn = ((u64) (param[2] & 0xff)) << 56;
+			mapx->wwn |= ((u64) ((param[2] >> 8) & 0xff)) << 48;
+			mapx->wwn |= ((u64) (param[3] & 0xff)) << 40;
+			mapx->wwn |= ((u64) ((param[3] >> 8) & 0xff)) << 32;
+			mapx->wwn |= ((u64) (param[6] & 0xff)) << 24;
+			mapx->wwn |= ((u64) ((param[6] >> 8) & 0xff)) << 16;
+			mapx->wwn |= ((u64) (param[7] & 0xff)) << 8;
+			mapx->wwn |= ((u64) ((param[7] >> 8) & 0xff));
 
 			j++;
-
+			mapx++;
 		}
 	}
 
 
 #if ISP2x00_FABRIC
-	isp2x00_init_fabric(host, temp, j);
+	isp2x00_init_fabric(host, map, j);
 #endif
 
-	for (i = 0; i <= QLOGICFC_MAX_ID; i++) {
-		if (temp[i].wwn != hostdata->port_db[i].wwn) {
-			for (j = 0; j <= QLOGICFC_MAX_ID; j++) {
-				if (temp[j].wwn == hostdata->port_db[i].wwn) {
-					hostdata->port_db[i].loop_id = temp[j].loop_id;
+	for (i = 0, mapx = map; i <= QLOGICFC_MAX_ID; i++, mapx++) {
+		struct id_name_map *tmap; /* second array entry pointer */
+		if (mapx->wwn != hostdata->port_db[i].wwn) {
+			for (j = 0, tmap = map; j <= QLOGICFC_MAX_ID; j++, tmap++) {
+				if (tmap->wwn == hostdata->port_db[i].wwn) {
+					hostdata->port_db[i].loop_id = tmap->loop_id;
 					break;
 				}
 			}
 			if (j == QLOGICFC_MAX_ID + 1)
-				hostdata->port_db[i].loop_id = temp[0].loop_id;
+				hostdata->port_db[i].loop_id = map->loop_id;
 
 			for (j = 0; j <= QLOGICFC_MAX_ID; j++) {
-				if (hostdata->port_db[j].wwn == temp[i].wwn || !hostdata->port_db[j].wwn) {
+				if (hostdata->port_db[j].wwn == mapx->wwn || !hostdata->port_db[j].wwn) {
 					break;
 				}
 			}
 			if (j == QLOGICFC_MAX_ID + 1)
 				printk("qlogicfc%d : Too many scsi devices, no more room in port map.\n", hostdata->host_id);
 			if (!hostdata->port_db[j].wwn) {
-				hostdata->port_db[j].loop_id = temp[i].loop_id;
-				hostdata->port_db[j].wwn = temp[i].wwn;
+				hostdata->port_db[j].loop_id = mapx->loop_id;
+				hostdata->port_db[j].wwn = mapx->wwn;
 			}
 		} else
-			hostdata->port_db[i].loop_id = temp[i].loop_id;
+			hostdata->port_db[i].loop_id = mapx->loop_id;
 
 	}
 
 	isp2x00_enable_irqs(host);
-
+	kfree(map);
+fini:
 	return 0;
 }
#1 lore raw
Quoted 3 lines · Randy.Dunlap [[email protected]] wrote:

Can anyone suggest way(s) to have the isp2x00_make_portdb() function called over and over again until it gets its job done?
Or does anyone even still use this driver?

We have been using the Feral as it covers both Qlogic SCSI and FC cards (both 2200 and 2300 series, the qlogicfc only handles 2200). I believe Andrew Morton is carrying the driver in the mm series of patches.

-andmike
--
Michael Anderson
[email protected]

#2 lore raw
Quoted 10 lines · Mike Anderson
Quoted 3 lines · Randy.Dunlap [[email protected]] wrote:

Can anyone suggest way(s) to have the isp2x00_make_portdb() function called over and over again until it gets its job done?
Or does anyone even still use this driver?

We have been using the Feral as it covers both Qlogic SCSI and FC cards (both 2200 and 2300 series, the qlogicfc only handles 2200). I believe Andrew Morton is carrying the driver in the mm series of patches.

Does it work? After an initial spurt of not-so-hot testing reports
everything went quiet and I'd assumed people had given up on it for the while.

#3 lore raw
Quoted 7 lines · Andrew Morton [[email protected]] wrote:
Quoted 6 lines · Mike Anderson
Quoted 3 lines · Randy.Dunlap [[email protected]] wrote:

Can anyone suggest way(s) to have the isp2x00_make_portdb() function called over and over again until it gets its job done?
Or does anyone even still use this driver?

Quoted 8 lines
Quoted 4 lines

We have been using the Feral as it covers both Qlogic SCSI and FC cards (both 2200 and 2300 series, the qlogicfc only handles 2200). I believe Andrew Morton is carrying the driver in the mm series of patches.

Does it work? After an initial spurt of not-so-hot testing reports
everything went quiet and I'd assumed people had given up on it for the while.

No we have not given up. It works, but we still have the outstanding issue that we can overdrive the card do to the combination of can_queue, device queue_depth and the ring size allocated for the card. A longer term fix is need for this which Matthew is aware of. In the short term some adjustments of these values will reduce the number of requeues that are happening and also keep the SCSI error handler for firing for timeouts.

-andmike
--
Michael Anderson
[email protected]