Re: [PATCH] cxl/region: Refactor logic around check_last_peer()

[Jim Harris <jim.harris@samsung.com>]

On Tue, Oct 24, 2023 at 07:49:47PM -0700, Dan Williams wrote:
> 
> It turns out this patch fails cxl-region-sysfs.sh.
> 
>     test/cxl-region-sysfs.sh: failed at line 94
>     
>     [   38.367581] check_last_peer: cxl region8: cxl_host_bridge.0:port4: mem5:decoder15.1 pos 4 mismatched peer mem7:decoder17.0
>     
> This patch looked so appetizing that I really wanted it to be a bug in
> the test and not a bug in this patch, but I think it is the latter.
> 
> > @@ -1111,20 +1111,17 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> >  
> >       cxlsd = to_cxl_switch_decoder(&cxld->dev);
> >       if (cxl_rr->nr_targets_set) {
> > -             int i, distance;
> > +             int i;
> >  
> >               /*
> > -              * Passthrough decoders impose no distance requirements between
> > -              * peers
> > +              * Check if this endpoint's dport is already in the
> > +              * switch decoder's target list, and if so check that
> > +              * it is positioned correctly based on the switch's
> > +              * interleave.
> >                */
> > -             if (cxl_rr->nr_targets == 1)
> > -                     distance = 0;
> > -             else
> > -                     distance = p->nr_targets / cxl_rr->nr_targets;
> 
> This calculation is essentially doing the "top-down" version of the
> "bottom-up" position calculation Alison introduced in her proposed region
> assembly fixes:
> 
>     for_each_parent_port(...)
>         pos = pos * parent_ways + parent_pos
> 
> So in a x8 region across 2x HBs with 2x switches per HB. The "distance"
> of peers at the switch level is 4. This change makes that 2.
> 
> Maybe the right conceptual cleanup is to still ditch this distance
> calculation based on "p->nr_targets / cxl_rr->nr_targets" and walk up
> from this port and multiply the local ways by all the ancestral ways,
> but as is this gets the answer with less steps (modulo all the work to
> build up @ep and @cxl_rr).

Thanks Dan. I'll run with that and push out a v2.