Another libata TODO item

Another libata TODO item

[Jeff Garzik]

Difficulty:  beginner / intermediate

Modern network drivers have a per-NIC list of debugging messages that 
can be enabled/disabled at runtime, implemented as a bitmask named 
'msg_enable' in each driver.  VERY useful for tracing specific events 
during debugging.  grep for 'msg_enable', 'netif_msg_', and 'NETIF_MSG_'.

To make libata debugging easier and more fine-grained, we should convert 
DPRINTK/VPRINTK calls in libata to code that looks like

	if (ata_msg_xxx(ap->msg_enable))
		printk(...)

The task involves:

* reviewing net driver msg_enable usage
* reviewing original netif_msg documentation by Donald Becker, at 
(scroll down)
http://www.scyld.com/pipermail/vortex/2001-November/001426.html

* designing a sliding scale of ever-more-verbose classes of messages, 
for libata and libata drivers
* design a method by which userspace may change the per-port msg_enable 
variable in libata

* implement the sliding scale as ATA_MSG_xxx / ata_msg_xxx()
* add msg_enable to struct ata_port
* implement method of setting ap->msg_enable via userspace
* convert messages in libata-core/libata-scsi
* convert messages in each driver
* add 'debug' module option to each driver, in a manner that duplicates 
net driver module options
* test!

Re: Another libata TODO item

[Christoph Hellwig]

On Wed, Aug 24, 2005 at 02:17:11AM -0400, Jeff Garzik wrote:
> 
> Difficulty:  beginner / intermediate
> 
> Modern network drivers have a per-NIC list of debugging messages that 
> can be enabled/disabled at runtime, implemented as a bitmask named 
> 'msg_enable' in each driver.  VERY useful for tracing specific events 
> during debugging.  grep for 'msg_enable', 'netif_msg_', and 'NETIF_MSG_'.
> 
> To make libata debugging easier and more fine-grained, we should convert 
> DPRINTK/VPRINTK calls in libata to code that looks like
> 
> 	if (ata_msg_xxx(ap->msg_enable))
> 		printk(...)

Would be nice if you could move that one up to the scsi layer and combine
it with the existing scsi core loglevel handling.

Re: Another libata TODO item

[Jeff Garzik]

Christoph Hellwig wrote:
> On Wed, Aug 24, 2005 at 02:17:11AM -0400, Jeff Garzik wrote:
>>To make libata debugging easier and more fine-grained, we should convert 
>>DPRINTK/VPRINTK calls in libata to code that looks like
>>
>>	if (ata_msg_xxx(ap->msg_enable))
>>		printk(...)
> 
> 
> Would be nice if you could move that one up to the scsi layer and combine
> it with the existing scsi core loglevel handling.


Doubtful.  libata's use of SCSI will become optional, so using SCSI 
logging throughout libata core would be inappropriate.  Block layer 
would be more reasonable.

In any case, I also contine to be skeptical of in-kernel logging subsystems.

	Jeff

Re: Another libata TODO item

[Roland Dreier]

    Jeff> In any case, I also contine to be skeptical of in-kernel
    Jeff> logging subsystems.

Aren't you proposing a libata logging subsystem?

 - R.

Re: Another libata TODO item

[Jeff Garzik]

Roland Dreier wrote:
>     Jeff> In any case, I also contine to be skeptical of in-kernel
>     Jeff> logging subsystems.
> 
> Aren't you proposing a libata logging subsystem?

Look at net drivers.  Theres no real infrastructure beyond bit tests and 
printks.  I wouldn't call that a subsystem, so, I wouldn't call this one 
such either.

	Jeff

Re: Another libata TODO item

[Roland Dreier]

    Jeff> Look at net drivers.  Theres no real infrastructure beyond
    Jeff> bit tests and printks.  I wouldn't call that a subsystem,
    Jeff> so, I wouldn't call this one such either.

Well, scsi_logging.h isn't much of a subsystem either.

 - R.

Re: Another libata TODO item

[Jeff Garzik]

Roland Dreier wrote:
>     Jeff> Look at net drivers.  Theres no real infrastructure beyond
>     Jeff> bit tests and printks.  I wouldn't call that a subsystem,
>     Jeff> so, I wouldn't call this one such either.
> 
> Well, scsi_logging.h isn't much of a subsystem either.

You obviously did not grep for CONFIG_SCSI_LOGGING :)

	Jeff

[RFC] libata debugging

[Borislav Petkov]

On Wednesday 24 August 2005 23:04, Borislav Petkov wrote:
Hi there,

Jeff asked for a volunteer a couple days ago on lkml to implement libata 
debugging similar to the netdev debugging (see thread "Another libata TODO 
item") and I decided to give it a try. Here's a forward message I sent to him 
with the loglevels I think should be there and some additional implementation 
details I think should be discussed first before any coding attempts. Any 
comments are greatly appreciated.

Regards,
Boris.

> Hi Jeff,
>
> after going through libata-core and libata-scsi.c here are the logging
> levels I think might be appropriate:
>
> ATA_MSG_DRV		0x0001
> ATA_MSG_INFO	0x0002
> ATA_MSG_PROBE 	0x0004
> ATA_MSG_WARN 	0x0008
> ATA_MSG_MALLOC	0x0010
> ATA_MSG_CTL 		0x0020
> ATA_MSG_INTR	0x0040
> ATA_MSG_ERR		0x0080
>
>
> ATA_MSG_DRV:
> Driver info messages such as identity, version and copyright info.
>
> ATA_MSG_INFO:
> a little more verbose device config messages (to replace all printk's with
> KERN_INFO loglevel)
>
> ATA_MSG_PROBE:
> interaction with the controller. (printing of (un-)supported features while
> configuring the device; features setting as in ata_dev_set_xfermode() for
> example)
>
> ATA_MSG_WARN:
> warning messages like
> printk(KERN_WARNING "ata%u: no PIO support\n", ap->id);
> for example.
>
> ATA_MSG_MALLOC:
> debugging allocation, deallocation and mapping of driver memory.
>
> ATA_MSG_CTL:
> highly verbose device control/hardware registers (for ex. 
> ata_dev_select()), resetting the controller, etc. To replace mostly the
> debugging DPRINTK's.
>
> ATA_MSG_INTR:
> interrupt handling
>
> ATA_MSG_ERR:
> fatal hardware and other errors
>
>
> It seems to me though that these debug levels do not have the level of
> granularity that libata should probably have, although I don't see more
> subgroupings besides maybe an additional loglevel strictly for SCSI/ATA
> commands sent to the controller.
>
> Should we also replace all those enter/exit debug statements -
> VPRINTK("ENTER\n"); - with something like
> VPRINTK("function_name():ENTER\n"); for more easly locating the place in
> the code that the problem happens while debugging.
>
> Should the userspace setting of debug levels be done maybe through /proc
> like the ide cdrom driver, for example: /proc/sys/dev/cdrom/debug, so
> writing the ATA_MSG_XXX unsigned value there would enable or disable
> debugging?
>
> I know that in open source you just do things but i still think it is
> better to run these by you first and then do the patches. :)
>
> Regards,
> Boris.

Re: [RFC] libata debugging

[Borislav Petkov]

HI Jeff,

another question: 
unsigned int ata_dev_classify(struct ata_taskfile *tf) 

in drivers/scsi/libata-core.c doesn't have access to an ata_port struct and it 
thus cannot be converted to the new ata_msg_xxx usage. However, this function 
has only two users which both have ata_port defined in their function bodies. 
Should I change the interface of ata_dev_classify from 

unsigned int ata_dev_classify(struct ata_taskfile *tf)

to 

unsigned int ata_dev_classify(struct ata_taskfile *tf, struct ata_port *ap)

so that it can be converted to the proper logging system? I'm asking not 
because it is open source but because you probably might have some other 
interface design considerations or concerns that I (mostly) can't know of and 
an interface change is not an option.

--
Regards,
Boris.