Re: [PATCH v10 12/17] cxl/pci: Unify CXL trace logging for CXL Endpoints and CXL Ports

[Alison Schofield]

On Fri, Jun 27, 2025 at 12:22:39PM +0000, Shiju Jose wrote:
> >-----Original Message-----
> >From: Terry Bowman <terry.bowman@xxxxxxx>
> >Sent: 26 June 2025 23:43
> >To: dave@xxxxxxxxxxxx; Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>;
> >dave.jiang@xxxxxxxxx; alison.schofield@xxxxxxxxx; dan.j.williams@xxxxxxxxx;
> >bhelgaas@xxxxxxxxxx; Shiju Jose <shiju.jose@xxxxxxxxxx>;
> >ming.li@xxxxxxxxxxxx; Smita.KoralahalliChannabasappa@xxxxxxx;
> >rrichter@xxxxxxx; dan.carpenter@xxxxxxxxxx;
> >PradeepVineshReddy.Kodamati@xxxxxxx; lukas@xxxxxxxxx;
> >Benjamin.Cheatham@xxxxxxx;
> >sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx; terry.bowman@xxxxxxx;
> >linux-cxl@xxxxxxxxxxxxxxx
> >Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx
> >Subject: [PATCH v10 12/17] cxl/pci: Unify CXL trace logging for CXL Endpoints
> >and CXL Ports
> >

big snip -

> >-);
> >-
> > TRACE_EVENT(cxl_aer_uncorrectable_error,
> >-	TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32
> >*hl),
> >-	TP_ARGS(cxlmd, status, fe, hl),
> >+	TP_PROTO(struct device *dev, u64 serial, u32 status, u32 fe,
> >+		 u32 *hl),
> >+	TP_ARGS(dev, serial, status, fe, hl),
> > 	TP_STRUCT__entry(
> >-		__string(memdev, dev_name(&cxlmd->dev))
> >-		__string(host, dev_name(cxlmd->dev.parent))
> >+		__string(name, dev_name(dev))
> >+		__string(parent, dev_name(dev->parent))
> 
> Hi Terry,
> 
> Thanks for considering the feedback given in v9 regarding the compatibility issue
> with the rasdaemon.
> https://lore.kernel.org/all/959acc682e6e4b52ac0283b37ee21026@xxxxxxxxxx/
> 
> Probably some confusion w.r.t the feedback.
> Unfortunately  TP_printk(...) is not an ABI that we need to keep stable, 
> it's this structure, TP_STRUCT__entry(..) , that matters to the rasdaemon.
> 

I'm not so sure you should be letting him off the hook for TP_printk ;)
It seems TP_printk should be kept aligned w TP_STRUCT_entry(). As a
user who often looks at TP_printk output, I'd say keep them all in
sync, and consider them ABI - ie. add to but don't modify.



> > 		__field(u64, serial)
> > 		__field(u32, status)
> > 		__field(u32, first_error)
> > 		__array(u32, header_log, CXL_HEADERLOG_SIZE_U32)
> > 	),
> > 	TP_fast_assign(
> >-		__assign_str(memdev);
> >-		__assign_str(host);
> >-		__entry->serial = cxlmd->cxlds->serial;
> >+		__assign_str(name);
> >+		__assign_str(parent);
> >+		__entry->serial = serial;
> > 		__entry->status = status;
> > 		__entry->first_error = fe;
> > 		/*
> >@@ -99,8 +72,8 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
> > 		 */
> > 		memcpy(__entry->header_log, hl, CXL_HEADERLOG_SIZE);
> > 	),
> >-	TP_printk("memdev=%s host=%s serial=%lld: status: '%s' first_error:
> >'%s'",
> >-		  __get_str(memdev), __get_str(host), __entry->serial,
> >+	TP_printk("memdev=%s host=%s serial=%lld status='%s'
> >first_error='%s'",
> >+		  __get_str(name), __get_str(parent), __entry->serial,
> > 		  show_uc_errs(__entry->status),
> > 		  show_uc_errs(__entry->first_error)
> > 	)

snip