Messages in this thread | | | Subject | Re: [PATCH] perf stat: Add hint for SMI cost measurement | From | "Liang, Kan" <> | Date | Thu, 25 Apr 2019 15:05:32 -0400 |
| |
On 4/25/2019 1:47 PM, Ingo Molnar wrote: > > * 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 ... >
Sure, I will update the messages and send out V2.
Thanks, Kan
| |