Messages in this thread | | | Date | Tue, 17 Jan 2012 10:37:11 +0800 | From | Xiao Guangrong <> | Subject | Re: [PATCH 3/3] KVM: perf: kvm events analysis tool |
| |
On 01/16/2012 06:08 PM, Stefan Hajnoczi wrote:
> On Mon, Jan 16, 2012 at 9:32 AM, Xiao Guangrong > <xiaoguangrong@linux.vnet.ibm.com> wrote: >> +DESCRIPTION >> +----------- >> +You can analyze some crucial events and statistics with this >> +'perf kvm-events' command. > > This line is very general and does not explain which events/statistics > can be collected or how you can use that information. I suggest > making this description more specific. Explain that this subcommand > observers kvm.ko tracepoints and annotates/decodes them with > additional information (this is why I would use this command and not > raw perf record -e kvm:\*). >
Okay.
>> + � � � { SVM_EXIT_MONITOR, � � � � � � � � � � "monitor" }, \ >> + � � � { SVM_EXIT_MWAIT, � � � � � � � � � � � "mwait" }, \ >> + � � � { SVM_EXIT_XSETBV, � � � � � � � � � � �"xsetbv" }, \ >> + � � � { SVM_EXIT_NPF, � � � � � � � � � � � � "npf" } > > All this copy-paste could be avoided by sharing this stuff with the > arch/x86/kvm/ code. >
I will try to combine them in the next version.
>> +static void exit_event_decode_key(struct event_key *key, char decode[20]) >> +{ >> + � � � const char *exit_reason = get_exit_reason(key->info, key->key); >> + >> + � � � memset(decode, 0, 20); >> + � � � strncpy(decode, exit_reason, 20); > > This is a bad pattern to follow when using strncpy(3) because if there > was a strlen(exit_reason) == 20 string then decode[] would not be > NUL-terminated. Right now it's safe but it's better to just use > strlcpy() and drop the memset(3). >
Good point.
>> +static void mmio_event_decode_key(struct event_key *key, char decode[20]) >> +{ >> + � � � memset(decode, 0, 20); >> + � � � sprintf(decode, "%#lx:%s", key->key, >> + � � � � � � � key->info == KVM_TRACE_MMIO_READ ? "R" : "W"); > > Please drop the memset and use snprintf(3) instead of sprintf(3). It > places the NUL-terminator and ensures you don't exceed the buffer > size. > > Same pattern below. >
Will do, thanks Stefan! :)
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |