All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Borislav Petkov <bbpetkov@yahoo.de>
Cc: linux-ide <linux-ide@vger.kernel.org>, Jeff Garzik <jgarzik@pobox.com>
Subject: Re: [PATCHSET] libata dbg scheme conversion, take 1
Date: Fri, 30 Jun 2006 02:03:11 +0900	[thread overview]
Message-ID: <44A407CF.4000508@gmail.com> (raw)
In-Reply-To: <20060629160912.GA23122@zmei.tnic>

Borislav Petkov wrote:
> This patchset converts the relevant pieces of libata to the new debugging
> scheme. Additionally, it adjusts the message level scheme to one appropriate for
> libata. 
> 
> There are still places which use the old debugging macros but they have to be
> dealt with on a case by case basis due to the functionality of the debugging
> macros. Although most of the levels should be appropriately mapped, there might
> still be some message level "skew" which can be dealt with in a later course.

Hello,

I think this patchset is looking good generally.  Please consider the 
following.

* Don't make currently visible-by-default messages invisible-by-default 
or vice-versa.

* Don't mix visiable-by-default messages with debug messages.  e.g. It 
seems you made ATA_MSG_INFO debug category and put EH messages and some 
debug messages into it.  This is not good.  EH messages should be 
visible to user by default && enabling it shouldn't turn on debug 
messages with it.

* Please create ATA_MSG_DEBUG category and put 
important-but-not-too-frequent debug messages into it.  e.g. The current 
patch puts all debug messages during probe into CMD.  Turning on CMD 
will generate a *lot* of messages and qc issues messages are sometimes 
very uninteresting.  OTOH, probing messages are generally low-volume but 
more interesting.  So, it should be possible to slect them separately. 
Just put not-too-frequent messages into DEBUG.

* To me, DRV/INFO distinction doesn't seem to be clear.

Thanks for the good work.  :)

-- 
tejun

  reply	other threads:[~2006-06-29 17:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-06-29 16:09 [PATCHSET] libata dbg scheme conversion, take 1 Borislav Petkov
2006-06-29 17:03 ` Tejun Heo [this message]
2006-06-29 17:18   ` Borislav Petkov
2006-06-29 17:42     ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44A407CF.4000508@gmail.com \
    --to=htejun@gmail.com \
    --cc=bbpetkov@yahoo.de \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.