Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751865AbdF0CnV (ORCPT ); Mon, 26 Jun 2017 22:43:21 -0400 Received: from szxga01-in.huawei.com ([45.249.212.187]:8801 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751520AbdF0CnN (ORCPT ); Mon, 26 Jun 2017 22:43:13 -0400 Subject: Re: [PATCH v5] trace: ras: add ARM processor error information trace event To: Steven Rostedt References: <1498275503-137890-1-git-send-email-xiexiuqi@huawei.com> <20170626093624.381a25cb@gandalf.local.home> CC: , , , , , , , , , , From: Xie XiuQi Message-ID: <918253cf-33ef-3152-a492-14f2b8946e99@huawei.com> Date: Tue, 27 Jun 2017 10:41:34 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170626093624.381a25cb@gandalf.local.home> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.19.210] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020206.5951C5F6.0045,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 76777ee99d118fff1162cf6928578763 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4548 Lines: 145 Hi Steve, Thanks for your comments. On 2017/6/26 21:36, Steven Rostedt wrote: > On Sat, 24 Jun 2017 11:38:23 +0800 > Xie XiuQi wrote: > >> diff --git a/include/linux/cper.h b/include/linux/cper.h >> index 4c671fc..17546bf 100644 >> --- a/include/linux/cper.h >> +++ b/include/linux/cper.h >> @@ -275,6 +275,11 @@ enum { >> #define CPER_ARM_INFO_FLAGS_PROPAGATED BIT(2) >> #define CPER_ARM_INFO_FLAGS_OVERFLOW BIT(3) >> >> +#define CPER_ARM_INFO_TYPE_CACHE 0 >> +#define CPER_ARM_INFO_TYPE_TLB 1 >> +#define CPER_ARM_INFO_TYPE_BUS 2 >> +#define CPER_ARM_INFO_TYPE_UARCH 3 >> + >> /* >> * All tables and structs must be byte-packed to match CPER >> * specification, since the tables are provided by the system BIOS >> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h >> index 429f46f..dd91ba8 100644 >> --- a/include/ras/ras_event.h >> +++ b/include/ras/ras_event.h >> @@ -206,6 +206,85 @@ >> __entry->running_state, __entry->psci_state) >> ); >> >> +#define ARM_PROC_ERR_TYPE \ >> + EM ( CPER_ARM_INFO_TYPE_CACHE, "cache error" ) \ >> + EM ( CPER_ARM_INFO_TYPE_TLB, "TLB error" ) \ >> + EM ( CPER_ARM_INFO_TYPE_BUS, "bus error" ) \ >> + EMe ( CPER_ARM_INFO_TYPE_UARCH, "micro-architectural error" ) > > These are all defines. As the name suggests, the TRACE_DEFINE_ENUM() is > for use with enums, not defines. You can nuke this part. > >> + >> +/* >> + * First define the enums in MM_ACTION_RESULT to be exported to userspace >> + * via TRACE_DEFINE_ENUM(). >> + */ >> +#undef EM >> +#undef EMe >> +#define EM(a, b) TRACE_DEFINE_ENUM(a); >> +#define EMe(a, b) TRACE_DEFINE_ENUM(a); >> + >> +ARM_PROC_ERR_TYPE >> + >> +/* >> + * Now redefine the EM() and EMe() macros to map the enums to the strings >> + * that will be printed in the output. >> + */ >> +#undef EM >> +#undef EMe >> +#define EM(a, b) { a, b }, >> +#define EMe(a, b) { a, b } > > All the EM* and friends above are not needed. The macro below will > translate into numbers not names in the format files in tracefs. So, do you mean we just use __print_symbolic like below: #define show_proc_err_type(type) \ __print_symbolic(type, \ { CPER_ARM_INFO_TYPE_CACHE, "cache error" }, \ { CPER_ARM_INFO_TYPE_TLB, "TLB error" }, \ { CPER_ARM_INFO_TYPE_BUS, "bus error" }, \ { CPER_ARM_INFO_TYPE_UARCH, "micro-architectural error" }) > > -- Steve > > >> + >> +#define show_proc_err_flags(flags) __print_flags(flags, "|", \ >> + { CPER_ARM_INFO_FLAGS_FIRST, "First error captured" }, \ >> + { CPER_ARM_INFO_FLAGS_LAST, "Last error captured" }, \ >> + { CPER_ARM_INFO_FLAGS_PROPAGATED, "Propagated" }, \ >> + { CPER_ARM_INFO_FLAGS_OVERFLOW, "Overflow" }) >> + >> +TRACE_EVENT(arm_err_info_event, >> + >> + TP_PROTO(const struct cper_arm_err_info *err), >> + >> + TP_ARGS(err), >> + >> + TP_STRUCT__entry( >> + __field(u8, type) >> + __field(u8, flags) >> + __field(u16, multiple_error) >> + __field(u64, error_info) >> + __field(u64, virt_fault_addr) >> + __field(u64, physical_fault_addr) >> + ), >> + >> + TP_fast_assign( >> + __entry->type = err->type; >> + memset(&__entry->flags, 0, >> + sizeof(*__entry) - offsetof(typeof(*__entry), flags)); >> + __entry->multiple_error = ~0; >> + >> + if (err->validation_bits & CPER_ARM_INFO_VALID_MULTI_ERR) >> + __entry->multiple_error = err->multiple_error; >> + >> + if (err->validation_bits & CPER_ARM_INFO_VALID_FLAGS) >> + __entry->flags = err->flags; >> + >> + if (err->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) >> + __entry->error_info = err->error_info; >> + >> + if (err->validation_bits & CPER_ARM_INFO_VALID_VIRT_ADDR) >> + __entry->virt_fault_addr = err->virt_fault_addr; >> + >> + if (err->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR) >> + __entry->physical_fault_addr = err->physical_fault_addr; >> + ), >> + >> + TP_printk("type: %s; count: %u; flags: %s;" >> + " error info: %016llx; virtual address: %016llx;" >> + " physical address: %016llx", >> + __print_symbolic(__entry->type, ARM_PROC_ERR_TYPE), >> + __entry->multiple_error, >> + show_proc_err_flags(__entry->flags), >> + __entry->error_info, __entry->virt_fault_addr, >> + __entry->physical_fault_addr) >> +); >> + >> /* >> * Non-Standard Section Report >> * > > > . > -- Thanks, Xie XiuQi