Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <53e1591ef288135f1dd803c15e971c96d06f54ba.camel@perches.com>
Subject: Re: [PATCH v2 2/2] KVM: LAPIC: remove the trailing newline used in
 the fmt parameter of TP_printk
From:   Joe Perches <joe@perches.com>
To:     Wanpeng Li <kernellwp@gmail.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Radim =?UTF-8?Q?Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>,
        Theodore Tso <tytso@mit.edu>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date:   Fri, 31 May 2019 11:57:04 -0700
In-Reply-To: <1559284814-20378-2-git-send-email-wanpengli@tencent.com>
References: <1559284814-20378-1-git-send-email-wanpengli@tencent.com>
         <1559284814-20378-2-git-send-email-wanpengli@tencent.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.30.1-1build1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 2019-05-31 at 14:40 +0800, Wanpeng Li wrote:
> The trailing newlines will lead to extra newlines in the trace file
[]
> diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
[]
> @@ -1365,7 +1365,7 @@ TRACE_EVENT(kvm_hv_timer_state,
>  			__entry->vcpu_id = vcpu_id;
>  			__entry->hv_timer_in_use = hv_timer_in_use;
>  			),
> -		TP_printk("vcpu_id %x hv_timer %x\n",
> +		TP_printk("vcpu_id %x hv_timer %x",
>  			__entry->vcpu_id,
>  			__entry->hv_timer_in_use)
>  );

Not about the kvm subsystem, but generically there are
many of these that could be removed.

$ git grep -w TP_printk | grep '\\n' | wc -l
45

Also, aren't all TP_printk formats supposed to be single line?

If not, these are odd as well.

$ git grep -w TP_printk | grep '\\n[^"]'
include/trace/events/9p.h:	    TP_printk("clnt %lu %s(tag = %d)\n%.3x: %16ph\n%.3x: %16ph\n",
net/tipc/trace.h:	TP_printk("%s\n%s", __get_str(header), __get_str(buf))
net/tipc/trace.h:	TP_printk("%s\n%s", __get_str(header), __get_str(buf))
net/tipc/trace.h:	TP_printk("<%u> %s\n%s%s", __entry->portid, __get_str(header),
net/tipc/trace.h:	TP_printk("<%s> %s\n%s", __entry->name, __get_str(header),
net/tipc/trace.h:	TP_printk("<%x> %s\n%s", __entry->addr, __get_str(header),

Perhaps the documentation files around these formats
	Documentation/trace/events.rst
	Documentation/trace/tracepoints.rst
could be improved as well.