[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20240905-fix_cxld-v2-1-51a520a709e4@quicinc.com>
Date: Thu, 05 Sep 2024 21:02:27 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Davidlohr Bueso <dave@...olabs.net>,
Jonathan Cameron <jonathan.cameron@...wei.com>,
Dave Jiang <dave.jiang@...el.com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
Dan Williams <dan.j.williams@...el.com>, Ben Widawsky <bwidawsk@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Zijun Hu <zijun_hu@...oud.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-cxl@...r.kernel.org,
linux-kernel@...r.kernel.org, Zijun Hu <quic_zijuhu@...cinc.com>,
stable@...r.kernel.org
Subject: [PATCH v2] cxl/region: Fix bad logic for finding a free switch cxl
decoder
From: Zijun Hu <quic_zijuhu@...cinc.com>
It is bad for current match_free_decoder()'s logic to find a free switch
cxl decoder as explained below:
- If all child decoders are sorted by ID in ascending order, then
current logic can be simplified as below one:
static int match_free_decoder(struct device *dev, void *data)
{
struct cxl_decoder *cxld;
if (!is_switch_decoder(dev))
return 0;
cxld = to_cxl_decoder(dev);
return cxld->region ? 0 : 1;
}
dev = device_find_child(&port->dev, NULL, match_free_decoder);
which does not also need to modify device_find_child()'s match data.
- If all child decoders are NOT sorted by ID in ascending order, then
current logic are wrong as explained below:
F: free, (cxld->region == NULL) B: busy, (cxld->region != NULL)
S(n)F : State of switch cxl_decoder with ID n is Free
S(n)B : State of switch cxl_decoder with ID n is Busy
Provided there are 2 child decoders: S(1)F -> S(0)B, then current logic
will fail to find a free decoder even if there are a free one with ID 1
Anyway, current logic is not good, fixed by finding a free switch cxl
decoder with minimal ID.
Fixes: 384e624bb211 ("cxl/region: Attach endpoint decoders")
Closes: https://lore.kernel.org/all/cdfc6f98-1aa0-4cb5-bd7d-93256552c39b@icloud.com/
Cc: stable@...r.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
---
Changes in v2:
- Correct title and commit message
- Link to v1: https://lore.kernel.org/r/20240903-fix_cxld-v1-1-61acba7198ae@quicinc.com
---
drivers/cxl/core/region.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 21ad5f242875..b9607b4fc40b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -797,21 +797,26 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos)
static int match_free_decoder(struct device *dev, void *data)
{
struct cxl_decoder *cxld;
- int *id = data;
+ struct cxl_decoder *target_cxld;
+ struct device **target_device = data;
if (!is_switch_decoder(dev))
return 0;
cxld = to_cxl_decoder(dev);
-
- /* enforce ordered allocation */
- if (cxld->id != *id)
+ if (cxld->region)
return 0;
- if (!cxld->region)
- return 1;
-
- (*id)++;
+ if (!*target_device) {
+ *target_device = get_device(dev);
+ return 0;
+ }
+ /* enforce ordered allocation */
+ target_cxld = to_cxl_decoder(*target_device);
+ if (cxld->id < target_cxld->id) {
+ put_device(*target_device);
+ *target_device = get_device(dev);
+ }
return 0;
}
@@ -839,8 +844,7 @@ cxl_region_find_decoder(struct cxl_port *port,
struct cxl_endpoint_decoder *cxled,
struct cxl_region *cxlr)
{
- struct device *dev;
- int id = 0;
+ struct device *dev = NULL;
if (port == cxled_to_port(cxled))
return &cxled->cxld;
@@ -849,7 +853,8 @@ cxl_region_find_decoder(struct cxl_port *port,
dev = device_find_child(&port->dev, &cxlr->params,
match_auto_decoder);
else
- dev = device_find_child(&port->dev, &id, match_free_decoder);
+ /* Need to put_device(@dev) after use */
+ device_for_each_child(&port->dev, &dev, match_free_decoder);
if (!dev)
return NULL;
/*
---
base-commit: 67784a74e258a467225f0e68335df77acd67b7ab
change-id: 20240903-fix_cxld-4f6575a90619
Best regards,
--
Zijun Hu <quic_zijuhu@...cinc.com>
Powered by blists - more mailing lists