Re: [PATCH 1/9] libata: change debugging macros/adjust dbg levels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

Generally looks good to me.  Just a few comments.

Borislav Petkov wrote:
change debugging macros ata_(port|dev)_printk and adjust debugging levels

Signed-off-by: <petkov@xxxxxxxxxxxxxxxxxxxx>


--- libata-dev/include/linux/libata.h	2006-06-29 17:17:50.000000000 +0200
+++ libata-dev/include/linux/libata.h.new	2006-06-28 17:23:30.000000000 +0200
@@ -67,24 +67,26 @@
 #define HAVE_LIBATA_MSG 1
enum {
-	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,
-};
-
-#define ata_msg_drv(p)    ((p)->msg_enable & ATA_MSG_DRV)
-#define ata_msg_info(p)   ((p)->msg_enable & ATA_MSG_INFO)
-#define ata_msg_probe(p)  ((p)->msg_enable & ATA_MSG_PROBE)
-#define ata_msg_warn(p)   ((p)->msg_enable & ATA_MSG_WARN)
-#define ata_msg_malloc(p) ((p)->msg_enable & ATA_MSG_MALLOC)
-#define ata_msg_ctl(p)    ((p)->msg_enable & ATA_MSG_CTL)
-#define ata_msg_intr(p)   ((p)->msg_enable & ATA_MSG_INTR)
-#define ata_msg_err(p)    ((p)->msg_enable & ATA_MSG_ERR)
+	ATA_MSG_ERR	= 0x0001,
+	ATA_MSG_WARN	= 0x0002,
+	ATA_MSG_DRV	= 0x0004,

Comment for ATA_MSG_DRV please. Also, I'm not sure we need separate DRV from INFO.

+	ATA_MSG_INFO	= 0x0008, /* revalidation messages, EH progress */
+	ATA_MSG_VDEBUG	= 0x0010, /* verbose hot path */
+	ATA_MSG_CMD	= 0x0020, /* issue / completion */
+	ATA_MSG_SG	= 0x0040, /* SG map/unmap handling */
+	ATA_MSG_TRACE	= 0x0080, /* function tracing, e.g. enter/exit */
+};
+
+const char *__ata_msg_lvs[] = {
+	[ATA_MSG_ERR]	 = KERN_ERR,
+	[ATA_MSG_WARN]	 = KERN_WARNING,
+	[ATA_MSG_DRV]	 = KERN_INFO,
+	[ATA_MSG_INFO]	 = KERN_INFO,
+	[ATA_MSG_VDEBUG] = KERN_DEBUG,
+	[ATA_MSG_CMD]	 = KERN_DEBUG,
+	[ATA_MSG_SG]	 = KERN_DEBUG,
+	[ATA_MSG_TRACE]	 = KERN_DEBUG
+};

Please declare the array here and define it in libata-core.c.

 static inline u32 ata_msg_init(int dval, int default_msg_enable_bits)
 {
@@ -805,11 +807,16 @@ extern void ata_do_eh(struct ata_port *a
 /*
  * printk helpers
  */
-#define ata_port_printk(ap, lv, fmt, args...) \
-	printk(lv"ata%u: "fmt, (ap)->id , ##args)
+#define ata_port_printk(ap, lv, fmt, args...) do { \
+	if (unlikely((ap)->msg_enable & (0xFF & (lv)))) \
                                         ^^^^why?

+		printk("%sata%u: "fmt, __ata_msg_lvs[lv], (ap)->id , ##args); \
+} while (0)
-#define ata_dev_printk(dev, lv, fmt, args...) \
-	printk(lv"ata%u.%02u: "fmt, (dev)->ap->id, (dev)->devno , ##args)
+#define ata_dev_printk(dev, lv, fmt, args...) do { \
+	if (unlikely((dev->ap)->msg_enable & (0xFF & (lv)))) \
                                              ^^^^ditto

+		printk("%sata%u.%02u: "fmt, __ata_msg_lvs[lv], \
+				(dev)->ap->id, (dev)->devno , ##args); \
+} while (0)
/*
  * ata_eh_info helpers
@@ -997,9 +1004,8 @@ static inline u8 ata_wait_idle(struct at
if (status & (ATA_BUSY | ATA_DRQ)) {
 		unsigned long l = ap->ioaddr.status_addr;
-		if (ata_msg_warn(ap))
-			printk(KERN_WARNING "ATA: abnormal status 0x%X on port 0x%lX\n",
-				status, l);
+ ata_port_printk(ap, ATA_MSG_WARN, + "ATA: abnormal status 0x%X on port 0x%lX\n", status, l);
                         ^^^^^ you can drop the header.

--
tejun
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux