Re: [PATCH 2/4] cxl: Introduce to_{ram,pmem}_{res,perf}() helpers

[Dan Williams <dan.j.williams@intel.com>]

Alejandro Lucero Palau wrote:
> 
> On 1/17/25 06:10, Dan Williams wrote:
> > In preparation for consolidating all DPA partition information into an
> > array of DPA metadata, introduce helpers that hide the layout of the
> > current data. I.e. make the eventual replacement of ->ram_res,
> > ->pmem_res, ->ram_perf, and ->pmem_perf with a new DPA metadata array a
> > no-op for code paths that consume that information, and reduce the noise
> > of follow-on patches.
> >
> > The end goal is to consolidate all DPA information in 'struct
> > cxl_dev_state', but for now the helpers just make it appear that all DPA
> > metadata is relative to @cxlds.
> >
> > Note that a follow-on patch also cleans up the temporary placeholders of
> > @ram_res, and @pmem_res in the qos_class manipulation code,
> > cxl_dpa_alloc(), and cxl_mem_create_range_info().
> >
> > Cc: Dave Jiang <dave.jiang@intel.com>
> > Cc: Alejandro Lucero <alucerop@amd.com>
> > Cc: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
[..]
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index 2a25d1957ddb..78e92e24d7b5 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
[..]
> > @@ -438,13 +438,41 @@ struct cxl_dev_state {
> >   	bool rcd;
> >   	bool media_ready;
> >   	struct resource dpa_res;
> > -	struct resource pmem_res;
> > -	struct resource ram_res;
> > +	struct resource _pmem_res;
> > +	struct resource _ram_res;
> 
> 
> I think this is unnecessary since it is clear those fields are going 
> away later on, and this change only adds confusion. Moreover, they are 
> not referenced in the code now because the helpers.

That is part of demonstrating a safe conversion. You can read this
change to know that every possible usage of the old name is gone and
that is verified by the compiler.

Otherwise the reviewer needs to spend effort to grep to see if all the
old usages are indeed gone.

> > +static inline resource_size_t cxl_ram_size(struct cxl_dev_state *cxlds)
> > +{
> > +	const struct resource *res = to_ram_res(cxlds);
> > +
> > +	if (!res)
> > +		return 0;
> 
> 
> This check is not needed now, and with the change in next patch, I think 
> it should not be needed either.
> 
> Do we need the distinction between, no ram or no pmem, and ram/pmem with 
> size 0?

This was also Jonathan's feedback. In v2 I am removing the distinction
that PMEM is always index-1 in the cxl_dpa_partition array.