| Message ID | 170568501456.1008395.7903809557943927970.stgit@djiang5-mobl3 |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Add support to report region access coordinates to numa nodes | expand |
Dave Jiang wrote: > Refactor the common code of combining coordinates in order to reduce code. > Create a new function cxl_cooordinates_combine() it combine two 'struct > access_coordinate'. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/cdat.c | 32 +++++++++++++++++++++++--------- > drivers/cxl/core/port.c | 18 ++---------------- > drivers/cxl/cxl.h | 4 ++++ > 3 files changed, 29 insertions(+), 25 deletions(-) > > diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c > index cd84d87f597a..4d542627d02d 100644 > --- a/drivers/cxl/core/cdat.c > +++ b/drivers/cxl/core/cdat.c > @@ -183,15 +183,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, > xa_for_each(dsmas_xa, index, dent) { > int qos_class; > > - dent->coord.read_latency = dent->coord.read_latency + > - c.read_latency; > - dent->coord.write_latency = dent->coord.write_latency + > - c.write_latency; > - dent->coord.read_bandwidth = min_t(int, c.read_bandwidth, > - dent->coord.read_bandwidth); > - dent->coord.write_bandwidth = min_t(int, c.write_bandwidth, > - dent->coord.write_bandwidth); > - > + cxl_coordinates_combine(&dent->coord, &dent->coord, &c); > dent->entries = 1; > rc = cxl_root->ops->qos_class(root_port, &dent->coord, 1, &qos_class); > if (rc != 1) > @@ -514,4 +506,26 @@ void cxl_switch_parse_cdat(struct cxl_port *port) > } > EXPORT_SYMBOL_NS_GPL(cxl_switch_parse_cdat, CXL); > > +/** > + * cxl_coordinates_combine - Combine the two input coordinates into the first > + * > + * @c1: first coordinate, to be written to > + * @c2: second coordinate > + */ > +void cxl_coordinates_combine(struct access_coordinate *out, > + struct access_coordinate *c1, > + struct access_coordinate *c2) > +{ > + if (c2->write_bandwidth) > + out->write_bandwidth = min(c1->write_bandwidth, > + c2->write_bandwidth); > + out->write_latency = c1->write_latency + c2->write_latency; > + > + if (c2->read_bandwidth) > + out->read_bandwidth = min(c1->read_bandwidth, > + c2->read_bandwidth); > + out->read_latency = c1->read_latency + c2->read_latency; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_coordinates_combine, CXL); There is no need for EXPORT_SYMBOL() when the definition and the only caller exist within the same compilation unit, cxl_core.o. However, given there is nothing "CXL" about this function it likely wants to move out of cxl_core.o if another caller ever arrives.
On 1/19/24 17:35, Dan Williams wrote: > Dave Jiang wrote: >> Refactor the common code of combining coordinates in order to reduce code. >> Create a new function cxl_cooordinates_combine() it combine two 'struct >> access_coordinate'. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/cxl/core/cdat.c | 32 +++++++++++++++++++++++--------- >> drivers/cxl/core/port.c | 18 ++---------------- >> drivers/cxl/cxl.h | 4 ++++ >> 3 files changed, 29 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c >> index cd84d87f597a..4d542627d02d 100644 >> --- a/drivers/cxl/core/cdat.c >> +++ b/drivers/cxl/core/cdat.c >> @@ -183,15 +183,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, >> xa_for_each(dsmas_xa, index, dent) { >> int qos_class; >> >> - dent->coord.read_latency = dent->coord.read_latency + >> - c.read_latency; >> - dent->coord.write_latency = dent->coord.write_latency + >> - c.write_latency; >> - dent->coord.read_bandwidth = min_t(int, c.read_bandwidth, >> - dent->coord.read_bandwidth); >> - dent->coord.write_bandwidth = min_t(int, c.write_bandwidth, >> - dent->coord.write_bandwidth); >> - >> + cxl_coordinates_combine(&dent->coord, &dent->coord, &c); >> dent->entries = 1; >> rc = cxl_root->ops->qos_class(root_port, &dent->coord, 1, &qos_class); >> if (rc != 1) >> @@ -514,4 +506,26 @@ void cxl_switch_parse_cdat(struct cxl_port *port) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_switch_parse_cdat, CXL); >> >> +/** >> + * cxl_coordinates_combine - Combine the two input coordinates into the first >> + * >> + * @c1: first coordinate, to be written to >> + * @c2: second coordinate >> + */ >> +void cxl_coordinates_combine(struct access_coordinate *out, >> + struct access_coordinate *c1, >> + struct access_coordinate *c2) >> +{ >> + if (c2->write_bandwidth) >> + out->write_bandwidth = min(c1->write_bandwidth, >> + c2->write_bandwidth); >> + out->write_latency = c1->write_latency + c2->write_latency; >> + >> + if (c2->read_bandwidth) >> + out->read_bandwidth = min(c1->read_bandwidth, >> + c2->read_bandwidth); >> + out->read_latency = c1->read_latency + c2->read_latency; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_coordinates_combine, CXL); > > There is no need for EXPORT_SYMBOL() when the definition and the only > caller exist within the same compilation unit, cxl_core.o. > > However, given there is nothing "CXL" about this function it likely wants > to move out of cxl_core.o if another caller ever arrives. It's mostly used by core/cdat.c but eventually also used by core/port.c. So it's all within the core. >
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c index cd84d87f597a..4d542627d02d 100644 --- a/drivers/cxl/core/cdat.c +++ b/drivers/cxl/core/cdat.c @@ -183,15 +183,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port, xa_for_each(dsmas_xa, index, dent) { int qos_class; - dent->coord.read_latency = dent->coord.read_latency + - c.read_latency; - dent->coord.write_latency = dent->coord.write_latency + - c.write_latency; - dent->coord.read_bandwidth = min_t(int, c.read_bandwidth, - dent->coord.read_bandwidth); - dent->coord.write_bandwidth = min_t(int, c.write_bandwidth, - dent->coord.write_bandwidth); - + cxl_coordinates_combine(&dent->coord, &dent->coord, &c); dent->entries = 1; rc = cxl_root->ops->qos_class(root_port, &dent->coord, 1, &qos_class); if (rc != 1) @@ -514,4 +506,26 @@ void cxl_switch_parse_cdat(struct cxl_port *port) } EXPORT_SYMBOL_NS_GPL(cxl_switch_parse_cdat, CXL); +/** + * cxl_coordinates_combine - Combine the two input coordinates into the first + * + * @c1: first coordinate, to be written to + * @c2: second coordinate + */ +void cxl_coordinates_combine(struct access_coordinate *out, + struct access_coordinate *c1, + struct access_coordinate *c2) +{ + if (c2->write_bandwidth) + out->write_bandwidth = min(c1->write_bandwidth, + c2->write_bandwidth); + out->write_latency = c1->write_latency + c2->write_latency; + + if (c2->read_bandwidth) + out->read_bandwidth = min(c1->read_bandwidth, + c2->read_bandwidth); + out->read_latency = c1->read_latency + c2->read_latency; +} +EXPORT_SYMBOL_NS_GPL(cxl_coordinates_combine, CXL); + MODULE_IMPORT_NS(CXL); diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 7a5eff45e1e2..1d3a04ef8b4f 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -2095,20 +2095,6 @@ bool schedule_cxl_memdev_detach(struct cxl_memdev *cxlmd) } EXPORT_SYMBOL_NS_GPL(schedule_cxl_memdev_detach, CXL); -static void combine_coordinates(struct access_coordinate *c1, - struct access_coordinate *c2) -{ - if (c2->write_bandwidth) - c1->write_bandwidth = min(c1->write_bandwidth, - c2->write_bandwidth); - c1->write_latency += c2->write_latency; - - if (c2->read_bandwidth) - c1->read_bandwidth = min(c1->read_bandwidth, - c2->read_bandwidth); - c1->read_latency += c2->read_latency; -} - /** * cxl_endpoint_get_perf_coordinates - Retrieve performance numbers stored in dports * of CXL path @@ -2142,7 +2128,7 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, * nothing to gather. */ while (iter && !is_cxl_root(to_cxl_port(iter->dev.parent))) { - combine_coordinates(&c, &dport->sw_coord); + cxl_coordinates_combine(&c, &c, &dport->sw_coord); c.write_latency += dport->link_latency; c.read_latency += dport->link_latency; @@ -2151,7 +2137,7 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, } /* Augment with the generic port (host bridge) perf data */ - combine_coordinates(&c, &dport->hb_coord[ACCESS_COORDINATE_LOCAL]); + cxl_coordinates_combine(&c, &c, &dport->hb_coord[ACCESS_COORDINATE_LOCAL]); /* Get the calculated PCI paths bandwidth */ pdev = to_pci_dev(port->uport_dev->parent); diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 0fc06455233d..0a0a121ee780 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -879,6 +879,10 @@ void cxl_switch_parse_cdat(struct cxl_port *port); int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, struct access_coordinate *coord); +void cxl_coordinates_combine(struct access_coordinate *out, + struct access_coordinate *c1, + struct access_coordinate *c2); + /* * Unit test builds overrides this to __weak, find the 'strong' version * of these symbols in tools/testing/cxl/.
Refactor the common code of combining coordinates in order to reduce code. Create a new function cxl_cooordinates_combine() it combine two 'struct access_coordinate'. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/cxl/core/cdat.c | 32 +++++++++++++++++++++++--------- drivers/cxl/core/port.c | 18 ++---------------- drivers/cxl/cxl.h | 4 ++++ 3 files changed, 29 insertions(+), 25 deletions(-)