Re: regarding ata_msg_*()

[Tejun Heo <htejun@gmail.com>]

Hello, Borislav.

Borislav Petkov wrote:
>> * By default, ATA_MSG_INFO is turned off, which means 
>> ata_dev_configure() doesn't print anything about newly detected and 
>> configured device, which isn't good (BTW, why is @print_info completely 
>> ignored in that function?  It's needed.  We don't want to print those 
>> messages when we're just revalidating devices.)  Unfortunately, if I 
>> enable ATA_MSG_INFO, I enable some of function ENTER/EXIT messages, too. 
>>  Bah...
> 
> These all are different debugging levels which I proposed leaning on D. Becker's
> mail, see linux-ide archives from 25 Aug 2005. I agree that the debugging levels
> are somewhat "off-course" but this was not the main concern when sending the
> patches. Instead, we aimed first at the complete conversion to the new scheme 
> and then reajusting dbbg levels, so this is that...

I'm sorry that I'm whining now not back then, but better late than 
never, I guess.

>> * In ata_dev_read_id(), ENTER message is CTL, what is CTL?  What makes 
> In this mail it says also what CTL means and since nobody opposed to that I went
> on preparing the patches.

I'll look that up, but whatever it is, please make it apparent in the 
code - please add some comments after constant definitions at the very 
least.

[--snip--]
> I guess you're right about the laziness/carelessness (no insinuations
>  whatsoever :)) factor with developers but I still 

I would be one of the first ones being lazy/careless.  Insinuations 
welcomed.  :-)

> think it is a good thing to have different debugging levels for different
> message semantics and messages origin like interrupts, mem allocation, hardware probing,

To some degree, I agree but for example you mentioned mem allocation as 
one of the categories.  The thing is that libata almost never allocates 
anything during normal operation.  Even hotplug/unplugs are performed 
w/o any memory allocation.  Allocations only occur during driver 
attachment and if allocation fails during that, there's nothing much to 
do than printing error message and giving up - no need for separate 
category.

I'm not saying debug message categorization is unnecessary.  It will be 
useful, but let's do it only on as-needed basis.  IMHO, that will lead 
to the least amount of confusion.

> etc. I think, though, it would be better to have the debugging scheme running
> first and then rehash and reconfigure which debugging levels are appropriate and
> enough for libata-dev...

My suggestion is to keep the current (pre-msg_enable) model for the 
first conversion and categorize debug messages further after that.  So, 
message categories will be...

ATA_MSG_ERR
ATA_MSG_WARN
ATA_MSG_INFO
-------------> all above are enabled by default
ATA_MSG_DEBUG
ATA_MSG_VDEBUG

Then, you have 1-to-1 mapping w/ the existing messages.  You can simply 
incorporate message enabled tests into ata_*_printk() functions and the 
conversion would be trivial.

After that's complete, we can diversify ATA_MSG_DEBUG and ATA_MSG_VDEBUG 
by separating out chatty ones out.  e.g. you can separate out SG 
mapping/unmapping (including padding) debug messages, which produce 
massive amount of logs when enabled, into ATA_MSG_SG or something. 
After several such separations, debug messages should be quite 
manageable && the categories wouldn't be too elaborate.

Thanks.

-- 
tejun