Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 16 Oct 2022 14:43:33 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Alison Schofield <alison.schofield@intel.com>,
        Vishal Verma <vishal.l.verma@intel.com>,
        "Ben Widawsky" <bwidawsk@kernel.org>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>
Subject: Re: [RFC V2 PATCH 02/11] cxl/mem: Implement Get Event Records command
Message-ID: <Y0x7BQ31M2gi3ldX@iweiny-desk3>
References: <20221010224131.1866246-1-ira.weiny@intel.com>
 <20221010224131.1866246-3-ira.weiny@intel.com>
 <20221015072840.5f31517b@rorschach.local.home>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221015072840.5f31517b@rorschach.local.home>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Lm3LDWWEqhThJ/k8BUsV0SZ9VXcbyyACtsjEfxauUFAmThb2H4yOAhtszsan?=
 =?us-ascii?Q?CUU5zJW1BJH2ZgwoOpEoMDyVsENYe06DKulZINDdExCIkpKCKYBkjhPGEOeK?=
 =?us-ascii?Q?Qp5MBZOvsRfU2Oj6FeGVJL0oy+u8dqnxo8yhkPYs4+tw28uxCRyrLC1NDiHs?=
 =?us-ascii?Q?0/THMVFI+Y9E2qixXbXGPeTrF90Fyx97Fpay6Sn/kMCiab5bJZRwugWyIcqX?=
 =?us-ascii?Q?DVnWFYsM/v3jPEvjErNztoYhY53qmvVMifADLz1v2u7QRhUotb+BhQlLkzOL?=
 =?us-ascii?Q?2EG8otGT9ejP5MkM90BLPmJAc759/aEqknZWlOoYMhoAhsoBERmQnDbkQUQt?=
 =?us-ascii?Q?I+sXEqacliyB5hMrM5wzHpzn2NAxzjxkUuwOSrfgzgOzVZwvZr4paN6xUPq9?=
 =?us-ascii?Q?jpcbd4SfQ2GVktyY1zZkzaX6Zs8+m0Wh7s3KLMxZ3zyIPFqYodOKN743h+O1?=
 =?us-ascii?Q?rlkvA6wgvcWIKPVtwuxhOkEtF83ilIpKHGVq5rgKS47xttxFD0BbN0kPlB8+?=
 =?us-ascii?Q?7lRePw/rHWjyAykVN3KaH3T8PP0pSqM8vT8vgZVqw9sy2ONShwAm+O2v4+5t?=
 =?us-ascii?Q?5BlIKR/AZDBotvQjN718JiEBsxR5yQzBOBrcy9TDCrhNlz7xL4D53zqvIb1L?=
 =?us-ascii?Q?bhPmIqRsfnbU1gZaZPnTb2N+2KNvLIFmUADWrK47dVE/0YnM/5f62G0XQSJk?=
 =?us-ascii?Q?ZO080WNJhwjp7Y+Ef3OjvCpXtXGO0TyTHTciZkQbGfIRfceBTaTcAED6mM6A?=
 =?us-ascii?Q?+j+rTOKds52o6NQJ5ve0Tynum66pj39FoOIkiK7uBPzWg9zObWbfQfuLXBym?=
 =?us-ascii?Q?ua8RklZ8Xbe1alzQZiIpGZVBaxof06zZ8NLFPUw0OR/NB3YUFFiRHaLto7av?=
 =?us-ascii?Q?g0W2FlXatTKNZQ4m+jgv5lcfJWF0Fj5SobZnZvGLRa0UfD0J60gT0XEc/6TE?=
 =?us-ascii?Q?ixBxiOhHnMo+/2u8rZao+AnT8pMDa81ww1aHOoFQzrGZn8oz7HPDR9GlPVWG?=
 =?us-ascii?Q?jqwlt5DQMQp99KdK7CDKBSXTZNpWGEGuNRQjKIEyFaX8GVwCI62vzZY+gMqv?=
 =?us-ascii?Q?wE2r5v5zs6yWRhIFVCWRFHXZzlgQyL1CzlWapPT3RrjPM5LMiTHOuXl4wE15?=
 =?us-ascii?Q?GoIzji63EMtPKX727VbfjtFD8X7PqZmJVAw06iIzSDt5x5W3ehc2B5IY6j68?=
 =?us-ascii?Q?rYC+vOsZWOvDAWXOFITjkmYSjBp72+8sA2oR9/VSj5xBZa62ELUAn/HRnKxl?=
 =?us-ascii?Q?VB7f9yxElDny1WlO9Ji7YdcXwE787c6UQx7zkTo1UKN9zyASpTltMxgObGX3?=
 =?us-ascii?Q?fd7w1fN6HgmiV/yfEPijqkyTb30+HTmIoQh20+zL++eg1UZlCz2NGHvWaget?=
 =?us-ascii?Q?rOMuewN+rHPQzCH0pcsY5Kp2fPh45GLglfUPUIopoWO7RfCnOnra1Zi3ntZc?=
 =?us-ascii?Q?Oo1frQNX7xKgwhVF7Tfk7BQF8IclXzm7keAyyUjaeM+iILCTWow6kQ4X8J7t?=
 =?us-ascii?Q?2pe9u5TWmLiuO3Svf1AnFLjdiJbDT0lwYp8HhbfnRjX9rBmNOgT4E1xab0JK?=
 =?us-ascii?Q?WBsqNGE209QaWnxl2d+sbY8QSDYCLYacj+J9QB54?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 02cd06c8-5c6c-4116-b12c-08daafbf7bf5
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2022 21:43:38.1378
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DtzFIxReseQ7EHnI/a6y6VDnccnnaHuskkvdKBR3qoPMt0pH/LMnJuPpOZ8rLk4VZ3Y6t6kvPxPOzxe6Xc75tw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6936
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Oct 15, 2022 at 07:28:40AM -0400, Steven Rostedt wrote:
> On Mon, 10 Oct 2022 15:41:22 -0700
> ira.weiny@intel.com wrote:
> 
> > new file mode 100644
> > index 000000000000..318ba5fe046e
> > --- /dev/null
> > +++ b/include/trace/events/cxl.h
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#undef TRACE_SYSTEM
> > +#define TRACE_SYSTEM cxl
> > +
> > +#if !defined(_CXL_TRACE_EVENTS_H) ||  defined(TRACE_HEADER_MULTI_READ)
> > +#define _CXL_TRACE_EVENTS_H
> > +
> > +#include <asm-generic/unaligned.h>
> > +#include <linux/tracepoint.h>
> > +
> > +TRACE_EVENT(overflow,
> 
> Please do not use generic names for grouped events. As most tooling can
> use the name and not associate it with the group, it makes it ambiguous
> for what event it wants to enable.
> 
> That is, call it "cxl_overflow" and not "overflow".

I did not realize that.[1]

[1] https://lore.kernel.org/linux-cxl/67c4e0da-87e3-44a1-1902-220dbc312700@intel.com/

> 
> > +
> > +	TP_PROTO(const char *dev_name, enum cxl_event_log_type log,
> > +		 struct cxl_get_event_payload *payload),
> > +
> > +	TP_ARGS(dev_name, log, payload),
> > +
> > +	TP_STRUCT__entry(
> > +		__string(dev_name, dev_name)
> > +		__field(int, log)
> > +		__field(u64, first)
> > +		__field(u64, last)
> > +		__field(u16, count)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		__assign_str(dev_name, dev_name);
> > +		__entry->log = log;
> > +		__entry->count = le16_to_cpu(payload->overflow_err_count);
> > +		__entry->first = le64_to_cpu(payload->first_overflow_timestamp);
> > +		__entry->last = le64_to_cpu(payload->last_overflow_timestamp);
> > +	),
> > +
> > +	TP_printk("%s: EVENT LOG OVERFLOW log=%s : %u records from %llu to %llu",
> > +		__get_str(dev_name), cxl_event_log_type_str(__entry->log),
> > +		__entry->count, __entry->first, __entry->last)
> > +
> > +);
> > +
> 
> > +TRACE_EVENT(generic_event,
> 
> Same here. It's a "cxl_generic_event" not a "generic_event" that will
> clash with any other subsystem that would (but shouldn't) use the same
> name.

Sure I'll change it back.

Thanks for setting us straight,
Ira

> 
> -- Steve
> 
> > +
> > +	TP_PROTO(const char *dev_name, enum cxl_event_log_type log,
> > +		 struct cxl_event_record_raw *rec),
> > +
> > +	TP_ARGS(dev_name, log, rec),
> > +
> > +	TP_STRUCT__entry(
> > +		CXL_EVT_TP_entry
> > +		__array(u8, data, CXL_EVENT_RECORD_DATA_LENGTH)
> > +	),
> > +
> > +	TP_fast_assign(
> > +		CXL_EVT_TP_fast_assign(dev_name, log, rec->hdr);
> > +		memcpy(__entry->data, &rec->data, CXL_EVENT_RECORD_DATA_LENGTH);
> > +	),
> > +
> > +	CXL_EVT_TP_printk("%s",
> > +		__print_hex(__entry->data, CXL_EVENT_RECORD_DATA_LENGTH))
> > +);
> > +
> > +#endif /* _CXL_TRACE_EVENTS_H */
> >