Skip to contentSkip to conversation

mpt fusion: Add logging support

PATCH NAK Prakash, Sathya 5 participants 11 messages Jul 24, 2007 – Aug 1, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover mpt fusion: Add logging support

The patches in this patch set adds support for logging facility that can be used to debug a number of Fusion MPT related problems.

The logging support can be enabled or disabled changing the kernel configuration flag CONFIF_FUSION_LOGGING

The debug level can be programmed on the fly via SysFS (hex values)
echo [level] > /sys/class/scsi_host/host#/debug_level
and also can be passed as module parameter mpt_debug_level for mptbase.ko

There are various debug levels that can be found in the source file mptdebug.h

Since the difference is of size 175 KB, it is split into five different patches grouped based on the files as described below.

[PATCH 1/5] : Changes for logging support in Kconfig, Makefile,
mptbase.h and addition of mptdebug.h
[PATCH 2/5] : Changes in mptbase.c for logging support [PATCH 3/5] : Changes in mptscsih.c for logging support [PATCH 4/5] : Changes in mptfc.c mptlan.c mptsas.c and mptspi.c
for logging support
[PATCH 5/5] : Changes in mptctl.c for logging support

---

#1 lore raw
Quoted 24 lines · Sathya

The patches in this patch set adds support for logging facility that can be used
to debug a number of Fusion MPT related problems.

The logging support can be enabled or disabled changing the kernel configuration flag CONFIF_FUSION_LOGGING

The debug level can be programmed on the fly via SysFS (hex values)
echo [level] > /sys/class/scsi_host/host#/debug_level
and also can be passed as module parameter mpt_debug_level for mptbase.ko

There are various debug levels that can be found in the source file mptdebug.h

Since the difference is of size 175 KB, it is split into five different patches
grouped based on the files as described below.

[PATCH 1/5] : Changes for logging support in Kconfig, Makefile,
mptbase.h and addition of mptdebug.h

(quoted diff) +0 -0
 	[PATCH 2/5] : Changes in mptbase.c for logging support
 	[PATCH 3/5] : Changes in mptscsih.c for logging support
 	[PATCH 4/5] : Changes in mptfc.c mptlan.c mptsas.c and mptspi.c
Quoted 3 lines

for logging support
[PATCH 5/5] : Changes in mptctl.c for logging support

These set of patchs are to replace the current cumbersome implementation of enabling debug defines in the driver Makefile, and recompiling the
driver. The new method allows the enduser to enable/disable debug
information any time without having to recompile or reboot the system. These code has been verified in the LSI test labs for several months, and there is no performance regressions with this. If there are no other further objections, please apply.

ACK

Eric Moore

#2 lore raw
In reply to Prakash, Sathya
Quoted 9 lines · Sathya

The patches in this patch set adds support for logging facility that can be used to debug a number of Fusion MPT related problems.

The logging support can be enabled or disabled changing the kernel configuration flag CONFIF_FUSION_LOGGING

The debug level can be programmed on the fly via SysFS (hex values)
echo [level] > /sys/class/scsi_host/host#/debug_level
and also can be passed as module parameter mpt_debug_level for mptbase.ko

The way your module parameter works is slightly counter intuitive. On all our other drivers, you can write a value into

/sys/module/<module>/parameters/<debug parameter>

And have it acted on immediately. In yours, it seems only to work before the host is probed (because after that, the value in the ioc structure is what's used).

The other question is are you really sure you actually want per host debugging? is the added flexibility in being able to turn it on and off per host worth the problems of explaining to the users where to find the parameter? I've got to bet that 95% of the installations only have a single fusion card anyway. would it not be simpler just to have a global module parameter that can be set and acted on from /sys/modules?

James

#3 lore raw
Quoted 9 lines · On Friday, July 27, 2007 10:21 AM, wrote:

The way your module parameter works is slightly counter intuitive. On all our other drivers, you can write a value into

/sys/module/<module>/parameters/<debug parameter>

And have it acted on immediately. In yours, it seems only to work before the host is probed (because after that, the value in the ioc structure is what's used).

not true, the debug parameter can be configured prior to the host being
probed. We have a command line option called mpt_debug_level, that
can set the debug level from mptbase.ko. That way you can enable certain debug during probe time prior to the loading of mptsas/mptfc/mptspi. Once those upper drivers are loaded, you can toggle off and on the debug via the shost_attrib. This is explained in mptdebug.h.

Quoted 11 lines

The other question is are you really sure you actually want per host debugging? is the added flexibility in being able to turn it on and off
per host worth the problems of explaining to the users where to find the
parameter? I've got to bet that 95% of the installations only have a single fusion card anyway. would it not be simpler just to have a global module parameter that can be set and acted on from /sys/modules?

I like having the added flexibility, and potential customers may agree. Our driver stack support multiple bus protocols, unlike other vendors, and some customers may ship fibre, sas, and spi in a single systems.. For my personal use, I like being able to have per host debugging, as I
have multiple cards in my systems. There are several cases I've
debugged two controller case, when boot OS is on one controller, and the debug efforts on another, in that case I only want to concern myself with the debug in question, not boot OS. The method of debug usesage is in mptdebug.h, so I would think people would look there, and figure it out. I also have a script below that sets all the host debug sas chips. Does this sound reasonable? If not, let me know.

---[Cut below]---
#!/bin/bash
#set -x
if (( $# != 1 )); then

	echo -e \\n"useage: set_debug_level <level>"
	echo -e \\n
	exit 1

fi

scsi_host="/sys/class/scsi_host"
cd ${scsi_host}
subfolders=ls -1
for i in ${subfolders}; do

	cd ${i}
	if [ `cat proc_name` != "mptsas" ]; then
		cd ${scsi_host}
		continue;
	fi;
	echo $1 > debug_level
	debug_level=`cat debug_level`
	echo for ${i} debug_level=$debug_level
	cd ${scsi_host}

done;
---[Cut above]---

#4 lore raw
Quoted 5 lines · Eric
Quoted 4 lines · On Friday, July 27, 2007 10:21 AM, wrote:

The way your module parameter works is slightly counter intuitive. On all our other drivers, you can write a value into

Quoted 8 lines
Quoted 5 lines

/sys/module/<module>/parameters/<debug parameter>

And have it acted on immediately. In yours, it seems only to work before the host is probed (because after that, the value in the ioc structure is what's used).

not true, the debug parameter can be configured prior to the host being probed.

That's what I just said ... if you mean can be configured *after* the host being probed, then I think the parameter needs a module_param_call() so you can intercept the set and update the ioc structures accordingly.

Quoted 6 lines

We have a command line option called mpt_debug_level, that
can set the debug level from mptbase.ko. That way you can enable certain debug during probe time prior to the loading of mptsas/mptfc/mptspi. Once those upper drivers are loaded, you can toggle off and on the debug via the shost_attrib. This is explained in mptdebug.h.

Yes, but my point was that most other module parameters are settable from /sys as well ... this one has some strange rules.

Quoted 12 lines
Quoted 11 lines

The other question is are you really sure you actually want per host debugging? is the added flexibility in being able to turn it on and off
per host worth the problems of explaining to the users where to find the
parameter? I've got to bet that 95% of the installations only have a single fusion card anyway. would it not be simpler just to have a global module parameter that can be set and acted on from /sys/modules?

Quoted 11 lines

I like having the added flexibility, and potential customers may agree. Our driver stack support multiple bus protocols, unlike other vendors, and some customers may ship fibre, sas, and spi in a single systems.. For my personal use, I like being able to have per host debugging, as I
have multiple cards in my systems. There are several cases I've
debugged two controller case, when boot OS is on one controller, and the debug efforts on another, in that case I only want to concern myself with the debug in question, not boot OS. The method of debug usesage is in mptdebug.h, so I would think people would look there, and figure it out. I also have a script below that sets all the host debug sas chips. Does this sound reasonable? If not, let me know.

OK fair enough ... I'm just pointing out it's non standard.

I really think the module parameter has to be hooked to act as a global setting, but otherwise, this looks fine.

James

#5 lore raw
Quoted 12 lines · James Bottomley
Quoted 11 lines · Eric
Quoted 9 lines · On Friday, July 27, 2007 10:21 AM, wrote:

The way your module parameter works is slightly counter intuitive. On all our other drivers, you can write a value into

/sys/module/<module>/parameters/<debug parameter>

And have it acted on immediately. In yours, it seems only to work before the host is probed (because after that, the value in the ioc structure is what's used).

Quoted 7 lines
Quoted 2 lines

not true, the debug parameter can be configured prior to the host being probed.

That's what I just said ... if you mean can be configured *after* the host being probed, then I think the parameter needs a module_param_call() so you can intercept the set and update the ioc structures accordingly.

I tell you what, let me just show you the actual patch. This allows you to write to the /sys/module/mptbase/parameters/mpt_debug_level and have it take effect in every ioc.

James

drivers/message/fusion/mptbase.c +16 -1
index e866dac..414c109 100644
@@ -88,7 +88,9 @@ module_param(mpt_channel_mapping, int, 0);
 MODULE_PARM_DESC(mpt_channel_mapping, " Mapping id's to channels (default=0)");
 
 static int mpt_debug_level;
-module_param(mpt_debug_level, int, 0);
+static int mpt_set_debug_level(const char *val, struct kernel_param *kp);
+module_param_call(mpt_debug_level, mpt_set_debug_level, param_get_int,
+		  &mpt_debug_level, 0600);
 MODULE_PARM_DESC(mpt_debug_level, " debug level - refer to mptdebug.h - (default=0)");
 
 #ifdef MFCNT
@@ -220,6 +222,19 @@ pci_enable_io_access(struct pci_dev *pdev)
 	pci_write_config_word(pdev, PCI_COMMAND, command_reg);
 }
 
+static int mpt_set_debug_level(const char *val, struct kernel_param *kp)
+{
+	int ret = param_set_int(val, kp);
+	MPT_ADAPTER *ioc;
+
+	if (ret)
+		return ret;
+
+	list_for_each_entry(ioc, &ioc_list, list)
+		ioc->debug_level = mpt_debug_level;
+	return 0;
+}
+
 /*
  *  Process turbo (context) reply...
  */
#6 lore raw
Quoted 7 lines · James Bottomley

I tell you what, let me just show you the actual patch. This allows you
to write to the /sys/module/mptbase/parameters/mpt_debug_level and have it take effect in every ioc.

ACK, If possible, I would like this patch thrown in with the other fusion logging patchs you added over the weekend to the scsi-rc-fixes-2.6 stream. Thanks.

On another note, while unloading the driver, and I get an following opps
from bsg in the context of scsi_remove_host. This is w/o the "SMP
passthrough" patch, so why would fusion drivers be linked to bsg? Woudn't this break mptspi and mptfc, since they will not be working with bsg.

Call Trace:
 [<ffffffff881f519c>] :scsi_transport_sas:sas_bsg_remove+0x27/0x32
 [<ffffffff881f6349>] :scsi_transport_sas:sas_host_remove+0x30/0x34
 [<ffffffff80375326>] transport_remove_classdev+0x1d/0x4c
 [<ffffffff80375001>] attribute_container_device_trigger+0x69/0xa7
 [<ffffffff8803778b>] :scsi_mod:scsi_remove_host+0xcd/0xfa
 [<ffffffff883461ac>] :mptscsih:mptscsih_remove+0x32/0xae
 [<ffffffff8031416d>] pci_device_remove+0x24/0x48
 [<ffffffff803720ed>] __device_release_driver+0x91/0xb3
 [<ffffffff80372631>] driver_detach+0xd6/0x115
 [<ffffffff80371ba3>] bus_remove_driver+0x6d/0x90
 [<ffffffff803143f4>] pci_unregister_driver+0x17/0x6b
 [<ffffffff88355044>] :mptsas:mptsas_exit+0x10/0x5f
 [<ffffffff80252afb>] sys_delete_module+0x1b1/0x1e0
 [<ffffffff80307d9c>] __up_write+0x21/0x10d
 [<ffffffff8020bc4e>] system_call+0x7e/0x83
#7 lore raw

From: "Moore, Eric" <[email protected]>
Subject: RE: [PATCH 0/5] mpt fusion: Add logging support
Date: Mon, 30 Jul 2007 12:33:00 -0600

Quoted 20 lines · James Bottomley
Quoted 7 lines

I tell you what, let me just show you the actual patch. This allows you
to write to the /sys/module/mptbase/parameters/mpt_debug_level and have it take effect in every ioc.

ACK, If possible, I would like this patch thrown in with the other fusion logging patchs you added over the weekend to the scsi-rc-fixes-2.6 stream. Thanks.

On another note, while unloading the driver, and I get an following opps
from bsg in the context of scsi_remove_host. This is w/o the "SMP
passthrough" patch, so why would fusion drivers be linked to bsg? Woudn't this break mptspi and mptfc, since they will not be working with bsg.

Quoted 16 lines
Call Trace:
 [<ffffffff881f519c>] :scsi_transport_sas:sas_bsg_remove+0x27/0x32
 [<ffffffff881f6349>] :scsi_transport_sas:sas_host_remove+0x30/0x34
 [<ffffffff80375326>] transport_remove_classdev+0x1d/0x4c
 [<ffffffff80375001>] attribute_container_device_trigger+0x69/0xa7
 [<ffffffff8803778b>] :scsi_mod:scsi_remove_host+0xcd/0xfa
 [<ffffffff883461ac>] :mptscsih:mptscsih_remove+0x32/0xae
 [<ffffffff8031416d>] pci_device_remove+0x24/0x48
 [<ffffffff803720ed>] __device_release_driver+0x91/0xb3
 [<ffffffff80372631>] driver_detach+0xd6/0x115
 [<ffffffff80371ba3>] bus_remove_driver+0x6d/0x90
 [<ffffffff803143f4>] pci_unregister_driver+0x17/0x6b
 [<ffffffff88355044>] :mptsas:mptsas_exit+0x10/0x5f
 [<ffffffff80252afb>] sys_delete_module+0x1b1/0x1e0
 [<ffffffff80307d9c>] __up_write+0x21/0x10d
 [<ffffffff8020bc4e>] system_call+0x7e/0x83

This patch fix the problem?

---
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] scsi_transport_sas: initialize sas_host_attrs->q

fix the bug to call bsg_unregister_queue for an uninitialized queue.

---
 drivers/scsi/scsi_transport_sas.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/scsi_transport_sas.c +1 -0
index 3120f4b..b0a21a2 100644
@@ -270,6 +270,7 @@ static int sas_host_setup(struct transport_container *tc, struct device *dev,
 	sas_host->next_target_id = 0;
 	sas_host->next_expander_id = 0;
 	sas_host->next_port_id = 0;
+	sas_host->q = NULL;
 
 	if (sas_bsg_initialize(shost, NULL))
 		dev_printk(KERN_ERR, dev, "fail to a bsg device %d\n",
Signature · 1 line
1.5.2.4
#8 lore raw
Quoted 28 lines · FUJITA Tomonori
Quoted 1 line

On another note, while unloading the driver, and I get an

following opps

Quoted 3 lines

from bsg in the context of scsi_remove_host. This is w/o the "SMP
passthrough" patch, so why would fusion drivers be linked to bsg? Woudn't this break mptspi and mptfc, since they will not be

working with

Quoted 19 lines

bsg.

Call Trace:
 [<ffffffff881f519c>] :scsi_transport_sas:sas_bsg_remove+0x27/0x32
 [<ffffffff881f6349>] :scsi_transport_sas:sas_host_remove+0x30/0x34
 [<ffffffff80375326>] transport_remove_classdev+0x1d/0x4c
 [<ffffffff80375001>] attribute_container_device_trigger+0x69/0xa7
 [<ffffffff8803778b>] :scsi_mod:scsi_remove_host+0xcd/0xfa
 [<ffffffff883461ac>] :mptscsih:mptscsih_remove+0x32/0xae
 [<ffffffff8031416d>] pci_device_remove+0x24/0x48
 [<ffffffff803720ed>] __device_release_driver+0x91/0xb3
 [<ffffffff80372631>] driver_detach+0xd6/0x115
 [<ffffffff80371ba3>] bus_remove_driver+0x6d/0x90
 [<ffffffff803143f4>] pci_unregister_driver+0x17/0x6b
 [<ffffffff88355044>] :mptsas:mptsas_exit+0x10/0x5f
 [<ffffffff80252afb>] sys_delete_module+0x1b1/0x1e0
 [<ffffffff80307d9c>] __up_write+0x21/0x10d
 [<ffffffff8020bc4e>] system_call+0x7e/0x83

This patch fix the problem?

Quoted 7 lines

---
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] scsi_transport_sas: initialize sas_host_attrs->q

fix the bug to call bsg_unregister_queue for an uninitialized queue.

yes it does fix it.

I'm not sure if this has already been posted to lsml@, but please apply to scsi-rc-fixes-2.6.

Thanks,
Eric

#9 lore raw
Quoted 33 lines · Eric
Quoted 32 lines · FUJITA Tomonori
Quoted 1 line

On another note, while unloading the driver, and I get an

following opps

Quoted 3 lines

from bsg in the context of scsi_remove_host. This is w/o the "SMP
passthrough" patch, so why would fusion drivers be linked to bsg? Woudn't this break mptspi and mptfc, since they will not be

working with

Quoted 19 lines

bsg.

Call Trace:
 [<ffffffff881f519c>] :scsi_transport_sas:sas_bsg_remove+0x27/0x32
 [<ffffffff881f6349>] :scsi_transport_sas:sas_host_remove+0x30/0x34
 [<ffffffff80375326>] transport_remove_classdev+0x1d/0x4c
 [<ffffffff80375001>] attribute_container_device_trigger+0x69/0xa7
 [<ffffffff8803778b>] :scsi_mod:scsi_remove_host+0xcd/0xfa
 [<ffffffff883461ac>] :mptscsih:mptscsih_remove+0x32/0xae
 [<ffffffff8031416d>] pci_device_remove+0x24/0x48
 [<ffffffff803720ed>] __device_release_driver+0x91/0xb3
 [<ffffffff80372631>] driver_detach+0xd6/0x115
 [<ffffffff80371ba3>] bus_remove_driver+0x6d/0x90
 [<ffffffff803143f4>] pci_unregister_driver+0x17/0x6b
 [<ffffffff88355044>] :mptsas:mptsas_exit+0x10/0x5f
 [<ffffffff80252afb>] sys_delete_module+0x1b1/0x1e0
 [<ffffffff80307d9c>] __up_write+0x21/0x10d
 [<ffffffff8020bc4e>] system_call+0x7e/0x83

This patch fix the problem?

---
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] scsi_transport_sas: initialize sas_host_attrs->q

Quoted 8 lines
Quoted 3 lines

fix the bug to call bsg_unregister_queue for an uninitialized queue.

yes it does fix it.

I'm not sure if this has already been posted to lsml@, but please apply to scsi-rc-fixes-2.6.

The following patch should fix this problem:

http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-rc-fixes-2.6.git;a=commit;h=c000c43cf12090972fad0fbb621d78c2100d0373

#10 lore raw
In reply to Moore, Eric

Hello Eric ,

Quoted 10 lines · Eric
Quoted 8 lines · On Friday, July 27, 2007 10:21 AM, wrote:

The way your module parameter works is slightly counter intuitive. On all our other drivers, you can write a value into

/sys/module/<module>/parameters/<debug parameter>

And have it acted on immediately. In yours, it seems only to work before the host is probed (because after that, the value in the ioc structure is what's used).

Quoted 7 lines

not true, the debug parameter can be configured prior to the host being
probed. We have a command line option called mpt_debug_level, that
can set the debug level from mptbase.ko. That way you can enable certain debug during probe time prior to the loading of mptsas/mptfc/mptspi. Once those upper drivers are loaded, you can toggle off and on the debug via the shost_attrib. This is explained in mptdebug.h.

	Where's the mptdebug.h file in linux-2.6.22.1 ? (*)

(*)
root@filesrv1:/usr/src # find linux-2.6.22.1/drivers/message/ -name mptdebug.h -print root@filesrv1:/usr/src #

I take it that that file is only available thru a git repo at this
time ? I know it's in the sources you shared with me , but ...
Another thing where is shost_attrib defined ? It's not even in the
sources you shared with me .

Isn't *.h files an odd place to put debug information for a sys admin
to find so s/he can find out howto debug their problem with a LSI device ?
Mind you that's just my opinion . But It sure would be nice to have
some relativlty decent document(s) available to this purpose in Documentation .

Quoted 44 lines
Quoted 9 lines

The other question is are you really sure you actually want per host debugging? is the added flexibility in being able to turn it on and off
per host worth the problems of explaining to the users where to find the
parameter? I've got to bet that 95% of the installations only have a single fusion card anyway. would it not be simpler just to have a global module parameter that can be set and acted on from /sys/modules?

I like having the added flexibility, and potential customers may agree. Our driver stack support multiple bus protocols, unlike other vendors, and some customers may ship fibre, sas, and spi in a single systems.. For my personal use, I like being able to have per host debugging, as I
have multiple cards in my systems. There are several cases I've
debugged two controller case, when boot OS is on one controller, and the debug efforts on another, in that case I only want to concern myself with the debug in question, not boot OS. The method of debug usesage is in mptdebug.h, so I would think people would look there, and figure it out. I also have a script below that sets all the host debug sas chips. Does this sound reasonable? If not, let me know.

---[Cut below]---
#!/bin/bash
#set -x
if (( $# != 1 )); then

	echo -e \\n"useage: set_debug_level <level>"
	echo -e \\n
	exit 1

fi

scsi_host="/sys/class/scsi_host"
cd ${scsi_host}
subfolders=ls -1
for i in ${subfolders}; do

	cd ${i}
	if [ `cat proc_name` != "mptsas" ]; then
		cd ${scsi_host}
		continue;
	fi;
	echo $1 > debug_level
	debug_level=`cat debug_level`
	echo for ${i} debug_level=$debug_level
	cd ${scsi_host}
Quoted 2 lines

done;
---[Cut above]---

Sure would be nice if this tool worked for scsi hosts instead of just
sas . At least that's the way it looks to me .

# ls -1 /sys/class/scsi_host
host0/
host1/
host2/
host3/
host4/
host5/
	The "if [ `cat proc_name` != "mptsas" ]; then ... fi" section seems to 
preclude the ability to set the debug level when using a non-mptsas host adapter .
	ie:
# cat host*/proc_name
aic79xx
aic79xx
mptspi
mptspi
mptspi
mptspi

Hth , JimL

ps: Might be nice to get your input back on the other notes I've sent .

Signature · 5 lines
+-----------------------------------------------------------------+
| James   W.   Laferriere | System   Techniques | Give me VMS     |
| Network        Engineer | 663  Beaumont  Blvd |  Give me Linux  |
| [email protected] | Pacifica, CA. 94044 |   only  on  AXP |
+-----------------------------------------------------------------+