iscsi: Kconfig option for debug prints.
PATCH 1/1 iscsi: Kconfig option for debug prints.
Remove the dark ages /* define debug_print */ in code, to use a Kconfig option. With a system like Kconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)
I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints for libiscsi_tcp & iscsi_tcp.
More iscsi drivers should use more bits.
--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)
drivers/scsi/Kconfig +15 -0
@@ -352,6 +352,21 @@ config ISCSI_TCP http://open-iscsi.org +config ISCSI_DEBUG + int "ISCSI debug prints" + depends on SCSI_ISCSI_ATTRS + default 0 + help + This is a bit-mask that turns some debug printing to Kernel's + Messages file. Each bit turns on another area of the code: + 1 - Turn on prints from iscsi libraries. + 2 - Turns on prints from iscsi_tcp operations. + Note to programmers: Use more bits in this bit-mask for other iscsi + drivers. + If you found a problem with ISCSI, please turn this on to + help us debug the problem. Send the Messages file plus problem + description to [email protected] mailing-list + source "drivers/scsi/cxgb3i/Kconfig" config SGIWD93_SCSI
drivers/scsi/iscsi_tcp.c +0 -7
@@ -48,13 +48,6 @@ MODULE_AUTHOR("Mike Christie <[email protected]>, " "Alex Aizman <[email protected]>");MODULE_DESCRIPTION ("iSCSI/TCP data-path" );MODULE_LICENSE ("GPL" ); -#undef DEBUG_TCP - -#ifdef DEBUG_TCP -#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) -#else -#define debug_tcp(fmt...) -#endif static struct scsi_transport_template *iscsi_sw_tcp_scsi_transport;static struct scsi_host_template iscsi_sw_tcp_sht;
drivers/scsi/iscsi_tcp.h +6 -0
@@ -25,6 +25,12 @@#include <scsi/libiscsi.h> #include <scsi/libiscsi_tcp.h> +#if (CONFIG_ISCSI_DEBUG &2 ) +#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) +#else +#define debug_tcp(fmt...) +#endif +struct socket ;struct iscsi_tcp_conn ;
drivers/scsi/libiscsi_tcp.c +0 -7
@@ -49,13 +49,6 @@ MODULE_AUTHOR("Mike Christie <[email protected]>, " "Alex Aizman <[email protected]>");MODULE_DESCRIPTION ("iSCSI/TCP data-path" );MODULE_LICENSE ("GPL" ); -#undef DEBUG_TCP - -#ifdef DEBUG_TCP -#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) -#else -#define debug_tcp(fmt...) -#endif static int iscsi_tcp_hdr_recv_done (struct iscsi_tcp_conn *tcp_conn,struct iscsi_segment *segment);
include/scsi/libiscsi.h +1 -2
@@ -45,8 +45,7 @@ struct iscsi_session;struct iscsi_nopin ;struct device ; -/* #define DEBUG_SCSI */ -#ifdef DEBUG_SCSI +#if (CONFIG_ISCSI_DEBUG &1 ) #define debug_scsi(fmt...) printk(KERN_INFO "iscsi: " fmt)#else #define debug_scsi(fmt...)
Signature · 1 line
1.6.0.1
drivers/scsi/Kconfig
Quoted 25 lines · Boaz Harrosh
Remove the dark ages /* define
debug_print*/ in code, to use aKconfigoption. With a system likeKconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints forlibiscsi_tcp&iscsi_tcp.More iscsi drivers should use more bits.
Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)drivers/scsi/Kconfig +0
-0
Mike hi.
These are over latest Linus. Sorry I mixed up the branches. If they don't apply to your tree any more, I'll rebase.
I'm sending these because for too many times I submit some code with iscsi sources edits, because I forget to remove the edits for enabling prints. Please consider for inclusion.
Thanks
Boaz
Quoted 24 lines · Boaz Harrosh
Quoted 23 lines · Boaz Harrosh
Remove the dark ages /* define
debug_print*/ in code, to use aKconfigoption. With a system likeKconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints forlibiscsi_tcp&iscsi_tcp.More iscsi drivers should use more bits.
Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)
Quoted 7 lines
drivers/scsi/Kconfig
Quoted 1 line
drivers/scsi/Kconfig +0
-0Mike hi.
These are over latest Linus. Sorry I mixed up the branches. If they don't apply to your tree any more, I'll rebase.
It applies ok.
Quoted 4 lines
I'm sending these because for too many times I submit some code with iscsi sources edits, because I forget to remove the edits for enabling prints. Please consider for inclusion.
What about compile time vs load time? Olaf had the attached patch which does it at module load time. It is nicer for users, but I was just worried about maybe there being a perf change with all the new ifs? I was waiting to do some testing to see if there was and change, but did not get around to it. Is that too paranoid (probably other factors like our crappy locking will slow us down more than some ifs for printks right)?
Make iscsi debugging a module option
This patch adds a debug module parameter to both libiscsi and iscsi_tcp, allowing to toggle debug at runtime via /sys/modules/$name/parameters/debug
--- drivers/scsi/Kconfig | 9 +++++++++ drivers/scsi/iscsi_tcp.c | 13 +++++++++---- drivers/scsi/libiscsi.c | 10 ++++++++++ include/scsi/libiscsi.h | 11 +++++++---- 4 files changed, 35 insertions(+), 8 deletions(-)
drivers/scsi/iscsi_tcp.c +9 -4
@@ -48,13 +48,18 @@ MODULE_AUTHOR("Dmitry Yusupov <dmitry_yu "Alex Aizman <[email protected]>");MODULE_DESCRIPTION ("iSCSI/TCP data-path" );MODULE_LICENSE ("GPL" ); -#undef DEBUG_TCP#define DEBUG_ASSERT -#ifdef DEBUG_TCP -#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) +#ifdef CONFIG_SCSI_ISCSI_DEBUG +static int iscsi_tcp_debug =0 ; +#define debug_tcp(fmt...) do { \ + if (unlikely(iscsi_tcp_debug)) \ + printk(KERN_INFO "tcp: " fmt); \ + } while (0) + +module_param_named (debug, iscsi_tcp_debug, int,S_IRUGO |S_IWUSR );#else -#define debug_tcp(fmt...) +#define debug_tcp(fmt...) do { } while (0)#endif #ifndef DEBUG_ASSERT
drivers/scsi/libiscsi.c +10 -0
@@ -37,6 +37,16 @@#include <scsi/scsi_transport_iscsi.h> #include <scsi/libiscsi.h> +/* Debug flag for libiscsi - needs to be +* exported so other iscsi modules can make use +* of the debug_scsi() macro */ +#ifdef CONFIG_SCSI_ISCSI_DEBUG +int libiscsi_debug =0 ; +EXPORT_SYMBOL_GPL (libiscsi_debug); + +module_param_named (debug, libiscsi_debug, int,S_IRUGO |S_IWUSR ); +#endif +struct iscsi_session *class_to_transport_session (struct iscsi_cls_session *cls_session) {
include/scsi/libiscsi.h +7 -4
@@ -41,11 +41,14 @@ struct iscsi_cls_conn;struct iscsi_session ;struct iscsi_nopin ; -/* #define DEBUG_SCSI */ -#ifdef DEBUG_SCSI -#define debug_scsi(fmt...) printk(KERN_INFO "iscsi: " fmt) +#ifdef CONFIG_SCSI_ISCSI_DEBUG +extern int libiscsi_debug; +#define debug_scsi(fmt...) do { \ + if (unlikely(libiscsi_debug)) \ + printk(KERN_INFO "iscsi: " fmt); \ + } while (0)#else -#define debug_scsi(fmt...) +#define debug_scsi(fmt...) do { } while (0)#endif #define ISCSI_DEF_XMIT_CMDS_MAX 128/* must be power of 2 */
drivers/scsi/Kconfig +9 -0
@@ -312,6 +312,15 @@ config ISCSI_TCP http://linux-iscsi.sf.net +config SCSI_ISCSI_DEBUG + bool "Support iSCSI debugging" + depends on ISCSI_TCP + default n + help + Say Y here to compile debugging support into libiscsi and + iscsi_tcp. Debugging will not be enable by default, but + can be switched on and off via module options. + config SGIWD93_SCSI tristate "SGI WD93C93 SCSI Driver" depends on SGI_IP22 && SCSI
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 39 lines · Mike Christie
Quoted 24 lines · Boaz Harrosh
Quoted 23 lines · Boaz Harrosh
Remove the dark ages /* define
debug_print*/ in code, to use aKconfigoption. With a system likeKconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints forlibiscsi_tcp&iscsi_tcp.More iscsi drivers should use more bits.
Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)Quoted 6 lines
drivers/scsi/Kconfig
Quoted 1 line
drivers/scsi/Kconfig +0
-0Mike hi.
These are over latest Linus. Sorry I mixed up the branches. If they don't apply to your tree any more, I'll rebase.
It applies ok.
Quoted 4 lines
I'm sending these because for too many times I submit some code with iscsi sources edits, because I forget to remove the edits for enabling prints. Please consider for inclusion.
Quoted 7 lines
What about compile time vs load time? Olaf had the attached patch which does it at module load time. It is nicer for users, but I was just worried about maybe there being a perf change with all the new ifs? I was waiting to do some testing to see if there was and change, but did not get around to it. Is that too paranoid (probably other factors like our crappy locking will slow us down more than some ifs for printks right)?
I don't think the performance matters because in his patch it is still commented out by a config SCSI_ISCSI_DEBUG set to n. So people can run as today. Though maybe make it an "int" option and not "bool" because this way allmodconfig will not turn it on. Note that all distros use allmodeconfig.
And if it is already an "int" it can be a bit-mask which is the default for a single iscsi module-param which also acts as a bit-mask for all iscsi drivers. Each driver has a bit.
Sure a module-param could be nice, but if we decide on the final API we can do a config only now, and add run-time later.
[And for me personally a config option is just good enough. Though I admit
that for a sysadmin run-time can be very nice]
But please let's do something. Current situation just keeps biting me on the a.. At just the times when I don't pay attention.
Tell me what you decide and I'll freshen up which ever patch you want. (Olaf patch has problems, mainly with code placements and that we added libiscsi_tcp.c, and my above proposition)
Thanks Mike
Boaz
A message that is not in the archive (sent off-list or not yet received): [email protected]
drivers/scsi/iscsi_tcp.h line 33
Quoted 12 lines · Boaz Harrosh
drivers/scsi/iscsi_tcp.h +6
-0@@ -25,6 +25,12 @@#include <scsi/libiscsi.h> #include <scsi/libiscsi_tcp.h> +#if (CONFIG_ISCSI_DEBUG &2 ) +#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) +#else +#define debug_tcp(fmt...) +#endif +
Hi!
Let me say that I feel that "tcp:" should be something like "iSCSI-TCP:", just to point out that it's related to iSCSI.
Regards,
Ulrich
Quoted 17 lines · Ulrich Windl
drivers/scsi/iscsi_tcp.h line 33
Quoted 12 lines · Boaz Harrosh
drivers/scsi/iscsi_tcp.h +6
-0@@ -25,6 +25,12 @@#include <scsi/libiscsi.h> #include <scsi/libiscsi_tcp.h> +#if (CONFIG_ISCSI_DEBUG &2 ) +#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) +#else +#define debug_tcp(fmt...) +#endif +Hi!
Quoted 7 lines
Let me say that I feel that "tcp:" should be something like "iSCSI-TCP:", just to point out that it's related to iSCSI.
Regards,
Ulrich
This is unrelated to this patch. If so, it should be in another, additional patch. I think we don't need to bother because these are in headers private to iscsi and should never be included by any other code.
But Mike if you want I can send search-replace patch for debug_tcp/debug_scsi macros. What do you prefer?
Thanks
Boaz
Quoted 30 lines · Boaz Harrosh
Remove the dark ages /* define
debug_print*/ in code, to use aKconfigoption. With a system likeKconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints forlibiscsi_tcp&iscsi_tcp.More iscsi drivers should use more bits.
Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)drivers/scsi/Kconfig +0
-0@@ -352,6 +352,21 @@ config ISCSI_TCP http://open-iscsi.org
drivers/scsi/Kconfig config ISCSI_TCP · line 363
Quoted 10 lines
drivers/scsi/Kconfig +9
-0+config ISCSI_DEBUG + int "ISCSI debug prints" + depends on SCSI_ISCSI_ATTRS + default 0 + help + This is a bit-mask that turns some debug printing to Kernel's + Messages file. Each bit turns on another area of the code: + 1 - Turn on prints from iscsi libraries. + 2 - Turns on prints from iscsi_tcp operations.
Is this bit 1, bit 2, or value 1, value 2? Not clear to me. If it's bit numbers, what about bit 0?
Quoted 76 lines
drivers/scsi/Kconfig +6
-0+ Note to programmers: Use more bits in this bit-mask for other iscsi + drivers. + If you found a problem with ISCSI, please turn this on to + help us debug the problem. Send the Messages file plus problem + description to [email protected] mailing-list + source "drivers/scsi/cxgb3i/Kconfig" config SGIWD93_SCSIdrivers/scsi/iscsi_tcp.c +0
-7@@ -48,13 +48,6 @@ MODULE_AUTHOR("Mike Christie <[email protected]>, " "Alex Aizman <[email protected]>");MODULE_DESCRIPTION ("iSCSI/TCP data-path" );MODULE_LICENSE ("GPL" ); -#undef DEBUG_TCP - -#ifdef DEBUG_TCP -#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) -#else -#define debug_tcp(fmt...) -#endif static struct scsi_transport_template *iscsi_sw_tcp_scsi_transport;static struct scsi_host_template iscsi_sw_tcp_sht;drivers/scsi/iscsi_tcp.h +6
-0@@ -25,6 +25,12 @@#include <scsi/libiscsi.h> #include <scsi/libiscsi_tcp.h> +#if (CONFIG_ISCSI_DEBUG &2 ) +#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) +#else +#define debug_tcp(fmt...) +#endif +struct socket ;struct iscsi_tcp_conn ;drivers/scsi/libiscsi_tcp.c +0
-7@@ -49,13 +49,6 @@ MODULE_AUTHOR("Mike Christie <[email protected]>, " "Alex Aizman <[email protected]>");MODULE_DESCRIPTION ("iSCSI/TCP data-path" );MODULE_LICENSE ("GPL" ); -#undef DEBUG_TCP - -#ifdef DEBUG_TCP -#define debug_tcp(fmt...) printk(KERN_INFO "tcp: " fmt) -#else -#define debug_tcp(fmt...) -#endif static int iscsi_tcp_hdr_recv_done (struct iscsi_tcp_conn *tcp_conn,struct iscsi_segment *segment);include/scsi/libiscsi.h +1
-2@@ -45,8 +45,7 @@ struct iscsi_session;struct iscsi_nopin ;struct device ; -/* #define DEBUG_SCSI */ -#ifdef DEBUG_SCSI +#if (CONFIG_ISCSI_DEBUG &1 ) #define debug_scsi(fmt...) printk(KERN_INFO "iscsi: " fmt)#else #define debug_scsi(fmt...)
Signature · 1 line
~Randy
Quoted 32 lines · Randy Dunlap
Quoted 31 lines · Boaz Harrosh
Remove the dark ages /* define
debug_print*/ in code, to use aKconfigoption. With a system likeKconfig, in code, commented out, configuration options are slavery and hard work.
(version control, manual edit ... need I say more)I've used an "int" config bit-mask so more areas of code can be selected with one Koption, but mainly so that allmodconfig will not turn it on.
bit-1 - will turn on prints for libiscsi.
bit-2 - will turn on prints forlibiscsi_tcp&iscsi_tcp.More iscsi drivers should use more bits.
Signed-off-by Boaz Harrosh <[email protected]>--- drivers/scsi/Kconfig | 15 +++++++++++++++ drivers/scsi/iscsi_tcp.c | 7 ------- drivers/scsi/iscsi_tcp.h | 6 ++++++ drivers/scsi/libiscsi_tcp.c | 7 ------- include/scsi/libiscsi.h | 3 +-- 5 files changed, 22 insertions(+), 16 deletions(-)drivers/scsi/Kconfig +0
-0@@ -352,6 +352,21 @@ config ISCSI_TCP http://open-iscsi.org
Quoted 14 lines
drivers/scsi/Kconfig config ISCSI_TCP · line 363
Quoted 9 lines
drivers/scsi/Kconfig +9
-0+config ISCSI_DEBUG + int "ISCSI debug prints" + depends on SCSI_ISCSI_ATTRS + default 0 + help + This is a bit-mask that turns some debug printing to Kernel's + Messages file. Each bit turns on another area of the code: + 1 - Turn on prints from iscsi libraries. + 2 - Turns on prints from iscsi_tcp operations.Is this bit 1, bit 2, or value 1, value 2? Not clear to me. If it's bit numbers, what about bit 0?
Yes you are right! Not clear at all I will fix it
<snip>
Boaz