Dan Williams wrote:
> Ira Weiny wrote:
> >
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> > index f433f4eae888..9ac323cbf195 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -729,7 +729,10 @@ static void cxl_cper_local_fn(struct work_struct *work)
> >
> > while (kfifo_out_spinlocked(&cxl_cper_fifo, &wd, 1,
> > &cxl_cper_read_lock)) {
> > - /* drop msg */
> > + struct cxl_cper_event_rec *rec = &wd.rec;
> > + union cxl_event *evt = &rec->event;
> > +
> > + trace_cper_cxl_gen_event(rec, &evt->generic);
>
> So it was confusing to read the empty stub function 2 patches back when this
> change was coming, and basic reporting of CXL event does not need the
> workqueue indirection. Note that EDAC triggers trace events directly in
> the atomic notifier chain, so CXL could do the same.
>
> > static DECLARE_WORK(cxl_local_work, cxl_cper_local_fn);
> > diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
> > index cbd3ddd7c33d..319faf552b65 100644
> > --- a/include/ras/ras_event.h
> > +++ b/include/ras/ras_event.h
>
> This is more heavywieght than I was expecting and defeats the purpose of
> centralizing advanced decode in the CXL driver itself.
Fair enough but it is not that heavy IMO. It was mostly lifted from the
CXL traces.
>
> I would expect this to be just the tracing equivalent of the
> ignore_section logic in cper_estatus_print_section().
I think it is fair to give the user some information. I'll redo this
patch to be first and drop the extra work item.
Ira
