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

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

Jonathan Cameron wrote:
> On Wed, 22 Jan 2025 00:59:21 -0800
> Dan Williams <dan.j.williams@intel.com> 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>
[..]
> >  	xa_for_each(dsmas_xa, index, dent) {
> > -		if (resource_size(&cxlds->ram_res) &&
> > -		    range_contains(&ram_range, &dent->dpa_range))
> > -			update_perf_entry(dev, dent, &mds->ram_perf);
> > -		else if (resource_size(&cxlds->pmem_res) &&
> > -			 range_contains(&pmem_range, &dent->dpa_range))
> > -			update_perf_entry(dev, dent, &mds->pmem_perf);
> > -		else
> > -			dev_dbg(dev, "no partition for dsmas dpa: %pra\n",
> > -				&dent->dpa_range);
> > +		for (int i = 0; i < ARRAY_SIZE(partition); i++) {
> > +			const struct resource *res = partition[i];
> > +			struct range range = {
> > +				.start = res->start,
> > +				.end = res->end,
> > +			};
> > +
> > +			if (range_contains(&range, &dent->dpa_range))
> > +				update_perf_entry(dev, dent, perf[i]);
> > +			else
> > +				dev_dbg(dev,
> > +					"no partition for dsmas dpa: %pra\n",
> > +					&dent->dpa_range);
> 
> This else branch looks less than helpful if I read the code right.
> It will fire at least once for every dsmas entry implying no partition when
> in reality it is it probably matched on next partition.
> Probably want to break out on match and check if i == ARRAY_SIZE(partition)
> after the for loop and only then print the message.

Ah, true, good catch.