From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH] trace-cmd: fix kvm_mmu_prepare_zap_page even name and kvm_mmu_get_page event output in kvm plugin Date: Thu, 27 Dec 2012 13:22:51 +0200 Message-ID: <20121227112251.GN17584@redhat.com> References: <20121225114631.GC17584@redhat.com> <1356530310.2750.1.camel@fedora> <20121226141302.GJ17584@redhat.com> <1356531752.2750.10.camel@fedora> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org To: Steven Rostedt Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57902 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751841Ab2L0LWx (ORCPT ); Thu, 27 Dec 2012 06:22:53 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBRBMr1I005399 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 27 Dec 2012 06:22:53 -0500 Content-Disposition: inline In-Reply-To: <1356531752.2750.10.camel@fedora> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Dec 26, 2012 at 09:22:32AM -0500, Steven Rostedt wrote: > 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. > Well, it isn't and if it were it would have prevented tarce-cmd running on new kernel from reading traces generated on older kernels. I will register both. > > > > 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. > Got it. -- Gleb.