| Message ID | 20240229002542.634982-1-dave.jiang@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] cxl: Remove checking of iter in cxl_endpoint_get_perf_coordinates() | expand |
Dave Jiang wrote: > The while() loop in cxl_endpoint_get_perf_coordinates() checks to see if > 'iter' is valid as part of the condition breaking out of the loop. However, > iter is being used before the check at the end of the while loop before > the next iteration starts. Given that the loop doesn't expect the iter to > be NULL because it stops before the root port, remove the iter check. This smells like a fix, but no "Fix" in the title or "Fixes" tag. So, is this a cleanup or a fix, and if it a fix what are the user visible effects of the bug?
On 2/28/24 5:32 PM, Dan Williams wrote: > Dave Jiang wrote: >> The while() loop in cxl_endpoint_get_perf_coordinates() checks to see if >> 'iter' is valid as part of the condition breaking out of the loop. However, >> iter is being used before the check at the end of the while loop before >> the next iteration starts. Given that the loop doesn't expect the iter to >> be NULL because it stops before the root port, remove the iter check. > > This smells like a fix, but no "Fix" in the title or "Fixes" tag. So, is > this a cleanup or a fix, and if it a fix what are the user visible > effects of the bug? I debated on this and in the end decided that it doesn't need to be a fix because there is no impact whether we check the iter or not in the loop since iter should not ever be NULL.
On Wed, 28 Feb 2024 17:25:41 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > The while() loop in cxl_endpoint_get_perf_coordinates() checks to see if > 'iter' is valid as part of the condition breaking out of the loop. However, > iter is being used before the check at the end of the while loop before > the next iteration starts. Given that the loop doesn't expect the iter to > be NULL because it stops before the root port, remove the iter check. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > drivers/cxl/core/port.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index e59d9d37aa65..e1d30a885700 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -2142,7 +2142,7 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, > * port each iteration. If the parent is cxl root then there is > * nothing to gather. > */ > - while (iter && !is_cxl_root(to_cxl_port(iter->dev.parent))) { > + while (!is_cxl_root(to_cxl_port(iter->dev.parent))) { > combine_coordinates(&c, &dport->sw_coord); > c.write_latency += dport->link_latency; > c.read_latency += dport->link_latency;
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index e59d9d37aa65..e1d30a885700 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -2142,7 +2142,7 @@ int cxl_endpoint_get_perf_coordinates(struct cxl_port *port, * port each iteration. If the parent is cxl root then there is * nothing to gather. */ - while (iter && !is_cxl_root(to_cxl_port(iter->dev.parent))) { + while (!is_cxl_root(to_cxl_port(iter->dev.parent))) { combine_coordinates(&c, &dport->sw_coord); c.write_latency += dport->link_latency; c.read_latency += dport->link_latency;
The while() loop in cxl_endpoint_get_perf_coordinates() checks to see if 'iter' is valid as part of the condition breaking out of the loop. However, iter is being used before the check at the end of the while loop before the next iteration starts. Given that the loop doesn't expect the iter to be NULL because it stops before the root port, remove the iter check. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- drivers/cxl/core/port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)