Re: [PATCH v4 2/2] cxl/edac: Mask DPA flags in the event record cache key

[Jongwon Lee]

Hi Ming,

Thanks for the careful review.

On 11/5/26 16:20, Li Ming wrote:
>> +static inline unsigned long cxl_rec_key(const struct cxl_event_media_hdr *hdr)
>> +{
>> +	return le64_to_cpu(hdr->phys_addr) & CXL_DPA_MASK;
>> +}
> This function actually returns the real DPA of the event record, I think
> the name should be cxl_rec_dpa()? xa_load() uses attrbs->dpa as the
> index, but xa_store() uses cxl_rec_key() as the index, that could make
> readers confused.

Agreed, cxl_rec_dpa() says what it returns. I'll rename it in v5.

> I think the implementation of skipping unrepairable records should be
> moved into a separate patch, and this patch focuses on using real DPA as
> the index parameter of the xarray.

Makes sense, I'll split it out. I would like to put that patch before
the masking one, though. Once the key is masked, the DPA from the trace
event matches a NOT_REPAIRABLE record, so if the masking went in first,
those records could be repaired at that commit.

> My first feeling is that we should not simply ignore unrepairable
> records without any logs. But I am not very familiar with this area, not
> sure if we should report this repairable bit in trace log or output a
> kernel log while finding an unrepairable record.

Good point. The trace events already carry the flag: cxl_general_media
and cxl_dram print dpa_flags='NOT_REPAIRABLE' for such a record, so user
space can see it before it requests a repair, and the request then fails
with -EINVAL. If the maintainers would like a message on that path as
well, I can add a dev_dbg() in v5.

I'll wait a few days for other comments before sending v5.

Thanks,
Jongwon