subject:"\[PATCH\] iscsi\: Kconfig option for debug prints."

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-02-09 Thread Mike Christie


Boaz Harrosh wrote:
> Mike Christie wrote:
>> Boaz Harrosh wrote:
>>> 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.
>>>
>> I made this a per module compile time option. I also modified the macors 
>> so they added a sessionX or connectionX:Y prefix.
> 
> Nice! in Israel when we want to say that something is highly desirable, we 
> say "America"
> 
> One small nitpicking comment, we might like to change this code to not
> compile at all if CONFIG_DEBUG_KERNEL is not defined something like:
> 
>   static unsigned int iscsi_max_lun = 512;
>   module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
>  
> + #ifdef CONFIG_DEBUG_KERNEL
>  +static int iscsi_sw_tcp_dbg;
>  +module_param_named(debug_iscsi_tcp, iscsi_sw_tcp_dbg, int,
>  +   S_IRUGO | S_IWUSR);
>  +MODULE_PARM_DESC(debug_iscsi_tcp, "Turn on debugging for iscsi_tcp module "
>  + "Set to 1 to turn on, and zero to turn off. Default is off.");
>  +
> + #else
> + static const int iscsi_sw_tcp_dbg = 0;
> + #endif /*CONFIG_DEBUG_KERNEL*/
>  +#define ISCSI_SW_TCP_DBG(_conn, dbg_fmt, arg...)\
>   ...
> 
> So the optimizer will discard all this code. Just as we have today.
> (The nice thing is that the code still gets compiled only not included in the 
> output)
> 
> What ever you decide I don't have any strong feelings about it.


Ok will look into. I have to fix up the patch due to your comments below.

> 
> Also
>> diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
>> index 9e3182e..c11b2e1 100644
>> --- a/include/scsi/libiscsi_tcp.h
>> +++ b/include/scsi/libiscsi_tcp.h
>> @@ -91,6 +91,16 @@ enum {
>>  ISCSI_TCP_SUSPENDED,/* conn is suspended */
>>  };
>>  
>> +
>> +#define ISCSI_DBG_TCP(_conn, dbg_fmt, arg...)   \
>> +do {\
>> +if (iscsi_dbg_libtcp)   \
>> +iscsi_conn_printk(KERN_INFO, _conn, \
>> + "%s " dbg_fmt, \
>> + __func__, ##arg);  \
>> +} while (0);
>> +
>> +
> 
> Are you sure you need this exported in header? It looks like it is only used
> in libiscsi_tcp.c? iscsi_tcp.c has it's own parameter.
> 
> But if you keep it at header I think it is best for future to also have
> an:
> +extern int iscsi_dbg_libtcp;
> like in libiscsi.h.
> 
> BTW same comment for ISCSI_DBG_SESSION/ISCSI_DBG_CONN are they used outside
> libiscsi.c ?
> 
>>  extern void iscsi_tcp_hdr_recv_prep(struct iscsi_tcp_conn *tcp_conn);
>>  extern int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct sk_buff *skb,
>>unsigned int offset, bool offloaded, int *status);


Ah yeah you are right. I had done this originally because I thought the 
LLD (cxgb3i, bnx2i, iscsi_tcp, iser) could use the debug helpers. But 
then I thought it became complicated for them because they do not know 
when to use the iscsi ones and when to use theirs. So for now I will 
punt on it and let the LLDs use their own.



>> -- 
>> 1.6.0.6
> 
> Thanks mike this is very nice I added this to my test scripts and it is very
> easy to turn on prints for debugging when problems happen.
> 
> if using module_param_named() does that means we can change that in runtime? 
> how?
> 

Yeah,

echo > /sys/module/iscsi module name/parameters/name of param

> Boaz
> 
> > 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-02-09 Thread Boaz Harrosh


Mike Christie wrote:
> Boaz Harrosh wrote:
>> 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.
>>
> 
> I made this a per module compile time option. I also modified the macors 
> so they added a sessionX or connectionX:Y prefix.

Nice! in Israel when we want to say that something is highly desirable, we say 
"America"

One small nitpicking comment, we might like to change this code to not
compile at all if CONFIG_DEBUG_KERNEL is not defined something like:

  static unsigned int iscsi_max_lun = 512;
  module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
 
+ #ifdef CONFIG_DEBUG_KERNEL
 +static int iscsi_sw_tcp_dbg;
 +module_param_named(debug_iscsi_tcp, iscsi_sw_tcp_dbg, int,
 + S_IRUGO | S_IWUSR);
 +MODULE_PARM_DESC(debug_iscsi_tcp, "Turn on debugging for iscsi_tcp module "
 +   "Set to 1 to turn on, and zero to turn off. Default is off.");
 +
+ #else
+ static const int iscsi_sw_tcp_dbg = 0;
+ #endif /*CONFIG_DEBUG_KERNEL*/
 +#define ISCSI_SW_TCP_DBG(_conn, dbg_fmt, arg...)  \
  ...

So the optimizer will discard all this code. Just as we have today.
(The nice thing is that the code still gets compiled only not included in the 
output)

What ever you decide I don't have any strong feelings about it.

Also
> diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
> index 9e3182e..c11b2e1 100644
> --- a/include/scsi/libiscsi_tcp.h
> +++ b/include/scsi/libiscsi_tcp.h
> @@ -91,6 +91,16 @@ enum {
>   ISCSI_TCP_SUSPENDED,/* conn is suspended */
>  };
>  
> +
> +#define ISCSI_DBG_TCP(_conn, dbg_fmt, arg...)\
> + do {\
> + if (iscsi_dbg_libtcp)   \
> + iscsi_conn_printk(KERN_INFO, _conn, \
> +  "%s " dbg_fmt, \
> +  __func__, ##arg);  \
> + } while (0);
> +
> +

Are you sure you need this exported in header? It looks like it is only used
in libiscsi_tcp.c? iscsi_tcp.c has it's own parameter.

But if you keep it at header I think it is best for future to also have
an:
+extern int iscsi_dbg_libtcp;
like in libiscsi.h.

BTW same comment for ISCSI_DBG_SESSION/ISCSI_DBG_CONN are they used outside
libiscsi.c ?

>  extern void iscsi_tcp_hdr_recv_prep(struct iscsi_tcp_conn *tcp_conn);
>  extern int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct sk_buff *skb,
> unsigned int offset, bool offloaded, int *status);
> -- 
> 1.6.0.6

Thanks mike this is very nice I added this to my test scripts and it is very
easy to turn on prints for debugging when problems happen.

if using module_param_named() does that means we can change that in runtime? 
how?

Boaz

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-02-08 Thread Mike Christie

Boaz Harrosh wrote:
> 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.
> 

I made this a per module compile time option. I also modified the macors 
so they added a sessionX or connectionX:Y prefix.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

>From 5e8e4a17f4fedf7eb86c180313b2a841e7cc764d Mon Sep 17 00:00:00 2001
From: Mike Christie 
Date: Sun, 8 Feb 2009 01:34:24 -0600

Make debugging a module load option.

---
 drivers/infiniband/ulp/iser/iscsi_iser.c |   10 +-
 drivers/scsi/iscsi_tcp.c |   57 +++-
 drivers/scsi/libiscsi.c  |  156 ++
 drivers/scsi/libiscsi_tcp.c  |  114 ++
 include/scsi/libiscsi.h  |   23 +++-
 include/scsi/libiscsi_tcp.h  |   10 ++
 6 files changed, 215 insertions(+), 155 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 1287639..4338f54 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -168,7 +168,7 @@ iscsi_iser_mtask_xmit(struct iscsi_conn *conn, struct 
iscsi_task *task)
 {
int error = 0;
 
-   debug_scsi("task deq [cid %d itt 0x%x]\n", conn->id, task->itt);
+   iser_dbg("task deq [cid %d itt 0x%x]\n", conn->id, task->itt);
 
error = iser_send_control(conn, task);
 
@@ -195,7 +195,7 @@ iscsi_iser_task_xmit_unsol_data(struct iscsi_conn *conn,
/* Send data-out PDUs while there's still unsolicited data to send */
while (iscsi_task_has_unsol_data(task)) {
iscsi_prep_data_out_pdu(task, r2t, &hdr);
-   debug_scsi("Sending data-out: itt 0x%x, data count %d\n",
+   iser_dbg("Sending data-out: itt 0x%x, data count %d\n",
   hdr.itt, r2t->data_count);
 
/* the buffer description has been passed with the command */
@@ -206,7 +206,7 @@ iscsi_iser_task_xmit_unsol_data(struct iscsi_conn *conn,
goto iscsi_iser_task_xmit_unsol_data_exit;
}
r2t->sent += r2t->data_count;
-   debug_scsi("Need to send %d more as data-out PDUs\n",
+   iser_dbg("Need to send %d more as data-out PDUs\n",
   r2t->data_length - r2t->sent);
}
 
@@ -227,12 +227,12 @@ iscsi_iser_task_xmit(struct iscsi_task *task)
if (task->sc->sc_data_direction == DMA_TO_DEVICE) {
BUG_ON(scsi_bufflen(task->sc) == 0);
 
-   debug_scsi("cmd [itt %x total %d imm %d unsol_data %d\n",
+   iser_dbg("cmd [itt %x total %d imm %d unsol_data %d\n",
   task->itt, scsi_bufflen(task->sc),
   task->imm_count, task->unsol_r2t.data_length);
}
 
-   debug_scsi("task deq [cid %d itt 0x%x]\n",
+   iser_dbg("task deq [cid %d itt 0x%x]\n",
   conn->id, task->itt);
 
/* Send the cmd PDU */
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 5a08ca4..9c2e527 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -48,13 +48,6 @@ MODULE_AUTHOR("Mike Christie , "
  "Alex Aizman ");
 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;
@@ -63,6 +56,21 @@ static struct iscsi_transport iscsi_sw_tcp_transport;
 static unsigned int iscsi_max_lun = 512;
 module_param_named(max_lun, iscsi_max_lun, uint, S_IRUGO);
 
+static int iscsi_sw_tcp_dbg;
+module_param_named(debug_iscsi_tcp, iscsi_sw_tcp_dbg, int,
+  S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(debug_iscsi_tcp, "Turn on debugging for iscsi_tcp module "
+"Set to 1 to turn on, and zero to turn off. Default is off.");
+
+#define ISCSI_SW_TCP_DBG(_conn, dbg_fmt, arg...)   \
+   do {

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-14 Thread Randy Dunlap


Boaz Harrosh wrote:
> 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.
> 
> Signed-off-by: Boaz Harrosh 
> ---
>  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(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index d25d21e..6ef42f6 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -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.

Is this bit 1, bit 2, or value 1, value 2?  Not clear to me.
If it's bit numbers, what about bit 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 open-iscsi@googlegroups.com mailing-list
> +
>  source "drivers/scsi/cxgb3i/Kconfig"
>  
>  config SGIWD93_SCSI
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index a566aa9..af092a8 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -48,13 +48,6 @@ MODULE_AUTHOR("Mike Christie , "
> "Alex Aizman ");
>  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;
> diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
> index ca6b7bc..1341b02 100644
> --- a/drivers/scsi/iscsi_tcp.h
> +++ b/drivers/scsi/iscsi_tcp.h
> @@ -25,6 +25,12 @@
>  #include 
>  #include 
>  
> +#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;
>  
> diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
> index 12354c5..4c9f827 100644
> --- a/drivers/scsi/libiscsi_tcp.c
> +++ b/drivers/scsi/libiscsi_tcp.c
> @@ -49,13 +49,6 @@ MODULE_AUTHOR("Mike Christie , "
> "Alex Aizman ");
>  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);
> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
> index 7360e19..2421c2a 100644
> --- a/include/scsi/libiscsi.h
> +++ b/include/scsi/libiscsi.h
> @@ -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...)


-- 
~Randy

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Boaz Harrosh


Randy Dunlap wrote:
> Boaz Harrosh wrote:
>> 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.
>>
>> Signed-off-by: Boaz Harrosh 
>> ---
>>  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(-)
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index d25d21e..6ef42f6 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -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.
> 
> 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



Boaz

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Boaz Harrosh

Mike Christie wrote:
> Boaz Harrosh wrote:
>> Boaz Harrosh wrote:
>>> 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.
>>>
>>> Signed-off-by: Boaz Harrosh 
>>> ---
>>>  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(-)
>>>
>>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> 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.
>>
> 
> It applies ok.
> 
>> 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)?
> 

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

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Mike Christie

Boaz Harrosh wrote:
> Boaz Harrosh wrote:
>> 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.
>>
>> Signed-off-by: Boaz Harrosh 
>> ---
>>  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(-)
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> 
> 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.
> 

It applies ok.

> 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)?

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

--- Begin Message ---

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

Signed-off-by: olaf.ki...@oracle.com
---
 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(-)

Index: iscsi-2.6/drivers/scsi/iscsi_tcp.c
===
--- iscsi-2.6.orig/drivers/scsi/iscsi_tcp.c
+++ iscsi-2.6/drivers/scsi/iscsi_tcp.c
@@ -48,13 +48,18 @@ MODULE_AUTHOR("Dmitry Yusupov ");
 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
Index: iscsi-2.6/drivers/scsi/libiscsi.c
===
--- iscsi-2.6.orig/drivers/scsi/libiscsi.c
+++ iscsi-2.6/drivers/scsi/libiscsi.c
@@ -37,6 +37,16 @@
 #include 
 #include 
 
+/* 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)
 {
Index: iscsi-2.6/include/scsi/libiscsi.h
===
--- iscsi-2.6.orig/include/scsi/libiscsi.h
+++ iscsi-2.6/include/scsi/libiscsi.h
@@ -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_MAX128 /* must be power of 2 */
Index: iscsi-2.6/drivers/scsi/Kconfig
===

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Boaz Harrosh


Ulrich Windl wrote:
> On 12 Jan 2009 at 17:33, Boaz Harrosh wrote:
> 
>> --- a/drivers/scsi/iscsi_tcp.h
>> +++ b/drivers/scsi/iscsi_tcp.h
>> @@ -25,6 +25,12 @@
>>  #include 
>>  #include 
>>  
>> +#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
> 
> 

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

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Ulrich Windl


On 12 Jan 2009 at 17:33, Boaz Harrosh wrote:

> --- a/drivers/scsi/iscsi_tcp.h
> +++ b/drivers/scsi/iscsi_tcp.h
> @@ -25,6 +25,12 @@
>  #include 
>  #include 
>  
> +#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


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Boaz Harrosh


Boaz Harrosh wrote:
> 
> 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.
> 
> Signed-off-by: Boaz Harrosh 
> ---
>  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(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig

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

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

[PATCH] iscsi: Kconfig option for debug prints.

2009-01-12 Thread Boaz Harrosh



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.

Signed-off-by: Boaz Harrosh 
---
 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(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index d25d21e..6ef42f6 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -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 open-iscsi@googlegroups.com mailing-list
+
 source "drivers/scsi/cxgb3i/Kconfig"
 
 config SGIWD93_SCSI
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index a566aa9..af092a8 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -48,13 +48,6 @@ MODULE_AUTHOR("Mike Christie , "
  "Alex Aizman ");
 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;
diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index ca6b7bc..1341b02 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -25,6 +25,12 @@
 #include 
 #include 
 
+#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;
 
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 12354c5..4c9f827 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -49,13 +49,6 @@ MODULE_AUTHOR("Mike Christie , "
  "Alex Aizman ");
 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);
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 7360e19..2421c2a 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -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...)
-- 
1.6.0.1


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to 
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

Re: [PATCH] iscsi: Kconfig option for debug prints.

[PATCH] iscsi: Kconfig option for debug prints.

11 matches

Site Navigation

Mail list logo

Footer information