| Message ID | 1415712477.22887.172.camel@sauron.fi.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Am 11.11.2014 um 14:27 schrieb Artem Bityutskiy: > On Tue, 2014-11-11 at 13:25 +0100, Richard Weinberger wrote: >> Am 11.11.2014 um 13:03 schrieb Artem Bityutskiy: >>> On Tue, 2014-11-11 at 09:15 +0100, Richard Weinberger wrote: >>>>> Do we really want the function name in every log message? >>>>> IMHO this is not wise except for pure debug logs. >>>> >>>> BTW: Why UBI-X? This looks odd. Either use UBIX or ubiX. >>> >>> How about something like this (untested): >>> >>> >>> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> >>> Date: Tue, 11 Nov 2014 13:56:34 +0200 >>> Subject: [PATCH] UBI: clean-up printing helpers >>> >>> Let's prefix UBI messages with 'ubiX' instead of 'UBI-X' - this is more >>> consistent with the way we name UBI devices. >>> >>> Also, commit "32608703 UBI: Extend UBI layer debug/messaging capabilities" >>> added the function name print to 'ubi_msg()' - lets revert this change, since >>> these messages are supposed to be just informative messages, and not debugging >>> messages. >> >> What is the benefit of having the function name still in ubi_warn() and ubi_err()? > > The benefit is that it is easier to find the source code where the > message comes from. And not necessarily because the message is > "cryptic", but because you may want to check the possible reasons of the > problem from the code. > >> e.g. >> [ 95.511825] ubi0 error: ubi_attach_mtd_dev: mtd0 is already attached to ubi0 >> >> If the log message is so cryptic that you need to lookup it in the source to understand it, >> we better fix the message. > > UBI messages are usually not cryptic, and I think we did try to keep > them user-friendly. If you hit a cryptic error or warning message, do > not hesitate to improve it please. Debug messages are often cryptic. That's my point. UBI has very good messages. From the message you know in most cases exactly what is going on and in what sub-system you have to look. > I am OK with removing function names from warnings and errors, though. > But they were there since the very beginning, and changing this is a > separate subject, so I'd prefer someone else to submit a corresponding > patch. If we keep the function names away from ubi_msg() I'm perfectly fine. :-) Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2014-11-11 at 20:49 +0100, Richard Weinberger wrote: > > I am OK with removing function names from warnings and errors, though. > > But they were there since the very beginning, and changing this is a > > separate subject, so I'd prefer someone else to submit a corresponding > > patch. > > If we keep the function names away from ubi_msg() I'm perfectly fine. :-) OK, I'm pushing the patch I sent. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index f80ffab..ee7ac0b 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -50,13 +50,13 @@ #define UBI_NAME_STR "ubi" /* Normal UBI messages */ -#define ubi_msg(ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \ - ubi->ubi_num, __func__, ##__VA_ARGS__) +#define ubi_msg(ubi, fmt, ...) pr_notice(UBI_NAME_STR "%d: " fmt "\n", \ + ubi->ubi_num, ##__VA_ARGS__) /* UBI warning messages */ -#define ubi_warn(ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \ +#define ubi_warn(ubi, fmt, ...) pr_warn(UBI_NAME_STR "%d warning: %s: " fmt "\n", \ ubi->ubi_num, __func__, ##__VA_ARGS__) /* UBI error messages */ -#define ubi_err(ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \ +#define ubi_err(ubi, fmt, ...) pr_err(UBI_NAME_STR "%d error: %s: " fmt "\n", \ ubi->ubi_num, __func__, ##__VA_ARGS__) /* Background thread name pattern */