Re: [kvm-ppc-devel] kvm trace support for ppc

[Tan, L. <li...@in...>]

-----Original Message-----
From: Hollis Blanchard [mailto:ho...@us...] 
Sent: 2008年5月15日 5:37
To: Tan, Li
Cc: kvm-devel; kvm...@li...
Subject: Re: kvm trace support for ppc

On Tuesday 13 May 2008 03:06:19 Tan, Li wrote:
> Hollisb,
> I have 2 more questions:
> 1. seems record won't be overwritten because current code is as following:
> /*
>  *  The relay channel is used in "no-overwrite" mode, it keeps trace of how
>  *  many times we encountered a full subbuffer, to tell user space app the
>  *  lost records there were.
>  */
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> 				     void *prev_subbuf, size_t prev_padding)
> {
> 	struct kvm_trace *kt;
> 
> 	if (!relay_buf_full(buf))
> 		return 1;
> 
> 	kt = buf->chan->private_data;
> 	atomic_inc(&kt->lost_records);
> 
> 	return 0;
> }
> 
> 2. Then needn't expose debugfs entry "kvmtrace-metadata", we can use 
existing relayfs to output struct metadata with kmagic, if we update code as 
following?
> static int kvm_subbuf_start_callback(struct rchan_buf *buf, void *subbuf,
> 				     void *prev_subbuf, size_t prev_padding)
> {
> 	struct kvm_trace *kt;
> 
> 	if (!relay_buf_full(buf))
> 	{
> 		if (!prev_subbuf) {
> 			//here is executed only once (when the channel is opened)
> 			subbuf_start_reserve(buf, sizeof(struct metadata));
> 			((struct metadata *)subbuf)->kmagic = 0x1234;
> 		}
> 		
> 		return 1;
> 	}
> 
> 	kt = buf->chan->private_data;
> 	atomic_inc(&kt->lost_records);
> 
> 	return 0;
> }

Ah, I didn't understand what the "lost records" handling was about. Given that 
it won't be lost, it would be OK for the kernel to export the header, and in 
that case I guess you would want it to be the same size as the other records. 
I'm not sure how I feel about that from a layering point of view, but at 
least it would be functional.

[tan] The relay channel is used in "no-overwrite" mode, so it will lost any new items if encounters a full subbuffer, "lost records" is to count lost records. Yes, metadata is the same size as the other records.

This solution needn't change kvmtrace user app, only need to change kvmtrace_format.pl like following:
        if i == 1:
            line = sys.stdin.read(struct.calcsize(HDRREC))
            (kmgc, umgc, mgcpad) = struct.unpack(HDRREC, line)

            if kmgc == 0x1234:
                rev = False
            else:
                rev = True
	    continue

	# If i > 1:
        line = sys.stdin.read(struct.calcsize(HDRREC))
	(event, pid, vcpu_id) = struct.unpack(HDRREC, line)
	...
	if rev:
		event = reverse_int(event)
		pid = reverse_int(pid)
		vcpu_id = reverse_int(vcpu_id)
	    tsc = reverse_qword(tsc)
	    d1 = reverse_int(d1)
	    d2 = reverse_int(d2)
	    d3 = reverse_int(d3)
	    d4 = reverse_int(d4)
	    d5 = reverse_int(d5)

 -- 
Hollis Blanchard
IBM Linux Technology Center