From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin Date: Wed, 26 Dec 2012 09:22:32 -0500 Message-ID: <1356531752.2750.10.camel@fedora> References: <20121225114631.GC17584@redhat.com> <1356530310.2750.1.camel@fedora> <20121226141302.GJ17584@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43658 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751151Ab2LZOWe (ORCPT ); Wed, 26 Dec 2012 09:22:34 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBQEMYVA014672 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 26 Dec 2012 09:22:34 -0500 In-Reply-To: <20121226141302.GJ17584@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, 2012-12-26 at 16:13 +0200, Gleb Natapov wrote: > On Wed, Dec 26, 2012 at 08:58:30AM -0500, Steven Rostedt wrote: > > On Tue, 2012-12-25 at 13:46 +0200, Gleb Natapov wrote: > > > kvm_mmu_zap_page even was renamed to kvm_mmu_prepare_zap_page. > > > Print out created field for kvm_mmu_get_page event. > > > > trace-cmd needs to be backward compatible with older kernels. If older > > kernels used kvm_mmu_zap_page, then please add a check for that and have > > the plugin cope with either one. > > > Something like this? > > if (pevent_find_event_by_name(pevent, "kvmmmu", "kvm_mmu_prepare_zap_page")) > pevent_register_event_handler(pevent, -1, "kvmmmu", > "kvm_mmu_prepare_zap_page", kvm_mmu_print_role, > NULL); > else > pevent_register_event_handler(pevent, -1, "kvmmmu", > "kvm_mmu_zap_page", kvm_mmu_print_role, NULL); Sure, if it works. > > Also when trace-cmd encounters an event it does not recognize it prints > mysterious message: > > trace-cmd: No such file or directory > bad op token { > > Is this a bug? > Yes and no ;-) I need to get rid of the "perror" part, for errors that don't set errno. That's been on my todo list for a long time. Maybe when I come back to work next week I'll fix that. The 'bad op token {' happens when it tries to parse an event and it comes across something that it doesn't recognize. In this case a '{'. The kvm events are notorious with having extremely complex "print_fmt" fields in their event format files. When no event handler is registered for an event, trace-cmd uses the print_fmt to figure out how to print it. This was never meant to be too complex of a parser. For example, looking at my kvm_mmu_prepare_zap_page, we have: print fmt: "%s", ({ const char *ret = p->buffer + p->len; static const char *access_str[] = { "---", "--x", "w--", "w-x", "-u-", "-ux", "wu-", "wux" }; union kvm_mmu_page_role role; role.word = REC->role; trace_seq_printf(p, "sp gfn %llx %u%s q%u%s %s%s" " %snxe root %u %s%c", REC->gfn, role.level, role.cr4_pae ? " pae" : "", role.quadrant, role.direct ? " direct" : "", access_str[role.access], role.invalid ? " invalid" : "", role.nxe ? "" : "!", REC->root_count, REC->unsync ? "unsync" : "sync", 0); ret; }) trace-cmd has no idea on how to parse "({ const char *ret ..." in fact it dies on that first "{" because trace-cmd is not a full C parser. This was why the plugins were created in the first place. To handle various events that have too complex print_fmts for trace-cmd to understand. Most events are simple print_fmts and do not need plugins. Like sock_rcvqueue_full: print fmt: "rmem_alloc=%d truesize=%u sk_rcvbuf=%d", REC->rmem_alloc, REC->truesize, REC->sk_rcvbuf trace-cmd has no problem parsing events like that. -- Steve