Re: [PATCH] perf stat: Add hint for SMI cost measurement

[Ingo Molnar <mingo@kernel.org>]

* Liang, Kan <kan.liang@linux.intel.com> wrote:

> 
> 
> On 4/25/2019 2:39 AM, Ingo Molnar wrote:
> > 
> > * kan.liang@linux.intel.com <kan.liang@linux.intel.com> wrote:
> > 
> > > +static void smi_env_check(void)
> > > +{
> > > +	char *name;
> > > +	size_t len;
> > > +
> > > +	if (sysfs__read_str(CPUIDLE_CUR_DRV, &name, &len)) {
> > > +		pr_warning("Failed to check cstate status.\n");
> > 
> > What a meaningless message. What did we want to do, what happened, and
> > why did it fail?
> > 
> > > +		return;
> > > +	}
> > > +
> > > +	if (strncmp(name, "none", 4)) {
> > > +		pr_warning("Cstate may cause drift between aperf and cycles. "
> > > +			   "Please completely disable cstate, "
> > > +			   "E.g. set idle=poll in grub\n");
> > 
> > Please keep user-visible strings in the same form that the user sees
> > them, i.e. in a single line.
> 
> To avoid the line over 80 characters, the quoted string was split across
> lines in the code. But the string is shown as a single line when it is
> output for user.
> 
> Are you suggesting to ignore the 80 characters rule when printing
> user-visible strings?
> Could you please confirm?

What I'd suggest is split it up into multiple messages, or shorten the 
message. Small violations of col80 for user visible strings is fine (up 
to 100-120 columns I guess), but if the string is longer it's a clear 
sign that the syslog message it too long to begin ...

Thanks,

	Ingo