Re: PATCH [2/8] scsi: megaraid_sas - add module param fast_load

Previous thread: [PATCH 1/8] scsi: megaraid_sas - add hibernation support by bo yang on Monday, October 1, 2007 - 8:40 am. (1 message)

Next thread: [PATCH 3/8] scsi: megaraid_sas - add module param max_sectors, cmd_per_lun by bo yang on Monday, October 1, 2007 - 8:51 am. (5 messages)
From: bo yang
Date: Monday, October 1, 2007 - 8:46 am

Driver will skip physical devices scan for the first time if the fast_load is set. This is to reduce time for loading driver.

Signed-off-by: Bo Yang <bo.yang@lsi.com>

---
 drivers/scsi/megaraid/megaraid_sas.c |   69 +++++++++++++++++++------
 1 files changed, 55 insertions(+), 14 deletions(-)

diff -uprN linux-2.6.22_orig/drivers/scsi/megaraid/megaraid_sas.c linux-2.6.22_new/drivers/scsi/megaraid/megaraid_sas.c
--- linux-2.6.22_orig/drivers/scsi/megaraid/megaraid_sas.c	2007-10-01 00:03:59.000000000 -0700
+++ linux-2.6.22_new/drivers/scsi/megaraid/megaraid_sas.c	2007-10-01 00:12:54.000000000 -0700
@@ -46,6 +46,22 @@
 #include <scsi/scsi_host.h>
 #include "megaraid_sas.h"
 
+/*
+ * Module parameters
+ */
+
+/*
+ * Fast driver load option, skip scanning for physical devices during
+ * load.  This would result in physical devices being skipped during
+ * driver load time. These can be later added though,
+ * using /proc/scsi/scsi
+ */
+static unsigned int fast_load;
+module_param_named(fast_load, fast_load, int, 0);
+MODULE_PARM_DESC(fast_load,
+	"megasas: Faster loading of the driver, skips physical devices! "\
+	"(default = 0)");
+
 MODULE_LICENSE("GPL");
 MODULE_VERSION(MEGASAS_VERSION);
 MODULE_AUTHOR("megaraidlinux@lsi.com");
@@ -1094,6 +1110,44 @@ megasas_service_aen(struct megasas_insta
 	megasas_return_cmd(instance, cmd);
 }
 
+static struct megasas_instance *megasas_lookup_instance(u16 host_no)
+{
+	int i;
+
+	for (i = 0; i < megasas_mgmt_info.max_index; i++) {
+		if ((megasas_mgmt_info.instance[i]) &&
+			(megasas_mgmt_info.instance[i]->host->host_no
+			== host_no))
+			return megasas_mgmt_info.instance[i];
+	}
+
+	return NULL;
+}
+
+static int megasas_slave_alloc(struct scsi_device *sdev)
+{
+	struct megasas_instance *instance;
+	int tmp_fastload = fast_load;
+
+	instance = megasas_lookup_instance(sdev->host->host_no);
+
+	if (tmp_fastload && sdev->channel < MEGASAS_MAX_PD_CHANNELS) {
+		if ((sdev->id == MEGASAS_MAX_DEV_PER_CHANNEL -1) ...
From: Christoph Hellwig
Date: Tuesday, October 30, 2007 - 10:36 am

/proc/scsi/scsi is deprecated, so please don't recommend it here.


no need for the braces here.  This should be:

		if (megasas_mgmt_info.instance[i] &&

This is a rather complicated and fragile way to implement this feature.

I'd recomment to change the call to scsi_scan_host in megasas_io_attach
to something like:

	if (fast_load) {
		int channel;

		for (channel = 0; channel < MEGASAS_MAX_PD_CHANNELS;
		     channel++) {
			scsi_scan_host_selected(host, channel, SCAN_WILD_CARD,
						SCAN_WILD_CARD, 0);
		}
	} else {
		scsi_scan_host(host);
	}

-

From: Yang, Bo
Date: Tuesday, November 6, 2007 - 12:04 pm

Re-Sending:

Christoph,

The e-mail I sent out on Oct. 31st was bounced back, I re-send it again.
 
I see that scsi_scan_host_selected is in scsi_priv.h and currently is
not used by any other driver. The scsi_priv.h is not part of the include
dir (/include/scsi). One of the major Linux distro's don't even include
this file in /usr/src/kernels. Also it looks like at this time this
function may not be available (not exported?) for driver modules to use.
Even if I include scsi_priv.h I get "unknown symbol" for this function
while loading.
May be in the long run we can solve all these issues to call
scsi_scan_host_selected. However, the current implementation works fine
and has been tested by LSI and others. This implementation doesn't break
any protocol nor does it adversely affect any driver functionality.

In a future update soon I will modify the comment on proc/scsi and
remove the extra braces you have pointed out. Please accept the current
implementation/patches. 

Regards, 
 
Bo Yang
------------------------------------------------------------------------
------------------------ 
From: Yang, Bo 
Sent: Wednesday, October 31, 2007 9:46 AM
To: Christoph Hellwig
Cc: linux-scsi@vger.kernel.org; James.Bottomley@SteelEye.com;
akpm@osdl.org; linux-kernel@vger.kernel.org; Patro, Sumant; Kolli, Neela
Subject: RE: PATCH [2/8] scsi: megaraid_sas - add module param fast_load


Christoph,

I see that scsi_scan_host_selected is in scsi_priv.h and currently is
not used by any other driver. The scsi_priv.h is not part of the include
dir (/include/scsi). One of the major Linux distro's don't even include
this file in /usr/src/kernels. Also it looks like at this time this
function may not be available (not exported?) for driver modules to use.
Even if I include scsi_priv.h I get "unknown symbol" for this function
while loading.
May be in the long run we can solve all these issues to call
scsi_scan_host_selected. However, the current implementation works fine
and has been ...
From: Christoph Hellwig
Date: Wednesday, November 7, 2007 - 10:39 am

Your implementation adds state to scanning that could easily break and
makes the driver complex for things that don't belong into a driver,
so there's a clear NACK for this from me.
-

From: Yang, Bo
Date: Friday, November 9, 2007 - 1:43 pm

James,

We are re-submitting those patches with the two patches (patch [2/8]--
"add module param fast_load" and patch [3/8] -- "add module param
max_sectors, cmd_per_lun" ) droped for now.  Later on we may implement
the module params as suggested by the community members.

Regards.

Bo Yang   


-----Original Message-----
From: Christoph Hellwig [mailto:hch@infradead.org] 
Sent: Wednesday, November 07, 2007 12:40 PM
To: Yang, Bo
Cc: Christoph Hellwig; linux-scsi@vger.kernel.org;
James.Bottomley@SteelEye.com; akpm@osdl.org;
linux-kernel@vger.kernel.org
Subject: Re: PATCH [2/8] scsi: megaraid_sas - add module param fast_load


functionality.

Your implementation adds state to scanning that could easily break and
makes the driver complex for things that don't belong into a driver, so
there's a clear NACK for this from me.
-

Previous thread: [PATCH 1/8] scsi: megaraid_sas - add hibernation support by bo yang on Monday, October 1, 2007 - 8:40 am. (1 message)

Next thread: [PATCH 3/8] scsi: megaraid_sas - add module param max_sectors, cmd_per_lun by bo yang on Monday, October 1, 2007 - 8:51 am. (5 messages)