| Message ID | 172895072669.39002.9296583943188706348.stgit@dwillia2-xfh.jf.intel.com |
|---|---|
| State | Accepted |
| Commit | ced502c17c327ddd874cd2edb3eb4b1ff05dceed |
| Headers | show |
| Series | cxl/port: Prevent out-of-order decoder allocation | expand |
On 2024/10/15 08:06, Dan Williams wrote: > With the recent change to allow out-of-order decoder de-commit it > highlights a need to strengthen the in-order decoder commit guarantees. > As it stands match_free_decoder() ensures that if 2 regions are racing > decoder allocations the one that wins the race will get the lower id > decoder, but that still leaves the race to *commit* the decoder. > > Rather than have this complicated case of "reserved in-order, but may > still commit out-of-order", just arrange for the reservation order to > match the commit-order. In other words, prevent subsequent allocations > until the last reservation is committed. > > This precludes overlapping region creation events and requires the > previous regionN to either move forward to the decoder commit stage or > drop its reservation before regionN+1 can move forward. That is, > provided that regionN and regionN+1 decode through the same switch port. > > As a side effect this allows match_free_decoder() to drop its dependency > on needing write access to the device_find_child() @data parameter [1]. > > Reported-by: Zijun Hu <zijun_hu@icloud.com> > Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] > Cc: Davidlohr Bueso <dave@stgolabs.net> > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Alison Schofield <alison.schofield@intel.com> > Cc: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > This patch is incremental to "cxl: Initialization and shutdown fixes" > > [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com > > drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 33 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 3478d2058303..dff618c708dc 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -778,26 +778,50 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos) > return rc; > } > > +static int check_commit_order(struct device *dev, const void *data) > +{ > + struct cxl_decoder *cxld = to_cxl_decoder(dev); > + > + /* > + * if port->commit_end is not the only free decoder, then out of > + * order shutdown has occurred, block further allocations until > + * that is resolved > + */ > + if (((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) > + return -EBUSY; > + return 0; > +} > + > static int match_free_decoder(struct device *dev, void *data) > { > + struct cxl_port *port = to_cxl_port(dev->parent); > struct cxl_decoder *cxld; > - int *id = data; > + int rc; > > if (!is_switch_decoder(dev)) > return 0; > > cxld = to_cxl_decoder(dev); > > - /* enforce ordered allocation */ > - if (cxld->id != *id) > + if (cxld->id != port->commit_end + 1) > return 0; > > - if (!cxld->region) > - return 1; > - > - (*id)++; > + if (cxld->region) { > + dev_dbg(dev->parent, > + "next decoder to commit (%s) is already reserved (%s)\n", > + dev_name(dev), dev_name(&cxld->region->dev)); > + return 0; > + } > > - return 0; > + rc = device_for_each_child_reverse_from(dev->parent, dev, NULL, > + check_commit_order); > + if (rc) { > + dev_dbg(dev->parent, > + "unable to allocate %s due to out of order shutdown\n", > + dev_name(dev)); > + return 0; > + } Above checking seems meaningless since there are only one CLDX with ID port->commit_end + 1. logical of below my proposal maybe what you want. https://lore.kernel.org/all/4239bfd4-d5fe-4ac8-a087-9e1584765e61@icloud.com/ > + return 1; > } > > static int match_auto_decoder(struct device *dev, void *data) > @@ -824,7 +848,6 @@ cxl_region_find_decoder(struct cxl_port *port, > struct cxl_region *cxlr) > { > struct device *dev; > - int id = 0; > > if (port == cxled_to_port(cxled)) > return &cxled->cxld; > @@ -833,7 +856,7 @@ 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); > + dev = device_find_child(&port->dev, NULL, match_free_decoder); > if (!dev) > return NULL; > /* >
Zijun Hu wrote: > On 2024/10/15 08:06, Dan Williams wrote: > > With the recent change to allow out-of-order decoder de-commit it > > highlights a need to strengthen the in-order decoder commit guarantees. > > As it stands match_free_decoder() ensures that if 2 regions are racing > > decoder allocations the one that wins the race will get the lower id > > decoder, but that still leaves the race to *commit* the decoder. > > > > Rather than have this complicated case of "reserved in-order, but may > > still commit out-of-order", just arrange for the reservation order to > > match the commit-order. In other words, prevent subsequent allocations > > until the last reservation is committed. > > > > This precludes overlapping region creation events and requires the > > previous regionN to either move forward to the decoder commit stage or > > drop its reservation before regionN+1 can move forward. That is, > > provided that regionN and regionN+1 decode through the same switch port. > > > > As a side effect this allows match_free_decoder() to drop its dependency > > on needing write access to the device_find_child() @data parameter [1]. > > > > Reported-by: Zijun Hu <zijun_hu@icloud.com> > > Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] > > Cc: Davidlohr Bueso <dave@stgolabs.net> > > Cc: Vishal Verma <vishal.l.verma@intel.com> > > Cc: Alison Schofield <alison.schofield@intel.com> > > Cc: Jonathan Cameron <jonathan.cameron@huawei.com> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > --- > > This patch is incremental to "cxl: Initialization and shutdown fixes" > > > > [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com > > > > drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- > > 1 file changed, 33 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index 3478d2058303..dff618c708dc 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c [..] > > Above checking seems meaningless since there are only one CLDX with ID > port->commit_end + 1. > > logical of below my proposal maybe what you want. > https://lore.kernel.org/all/4239bfd4-d5fe-4ac8-a087-9e1584765e61@icloud.com/ No. Consider that port->commit_end + 1 may walk off the end of available decoders.
On 2024/10/15 09:51, Dan Williams wrote: > Zijun Hu wrote: >> On 2024/10/15 08:06, Dan Williams wrote: >>> With the recent change to allow out-of-order decoder de-commit it >>> highlights a need to strengthen the in-order decoder commit guarantees. >>> As it stands match_free_decoder() ensures that if 2 regions are racing >>> decoder allocations the one that wins the race will get the lower id >>> decoder, but that still leaves the race to *commit* the decoder. >>> >>> Rather than have this complicated case of "reserved in-order, but may >>> still commit out-of-order", just arrange for the reservation order to >>> match the commit-order. In other words, prevent subsequent allocations >>> until the last reservation is committed. >>> >>> This precludes overlapping region creation events and requires the >>> previous regionN to either move forward to the decoder commit stage or >>> drop its reservation before regionN+1 can move forward. That is, >>> provided that regionN and regionN+1 decode through the same switch port. >>> >>> As a side effect this allows match_free_decoder() to drop its dependency >>> on needing write access to the device_find_child() @data parameter [1]. >>> >>> Reported-by: Zijun Hu <zijun_hu@icloud.com> >>> Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] >>> Cc: Davidlohr Bueso <dave@stgolabs.net> >>> Cc: Vishal Verma <vishal.l.verma@intel.com> >>> Cc: Alison Schofield <alison.schofield@intel.com> >>> Cc: Jonathan Cameron <jonathan.cameron@huawei.com> >>> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >>> --- >>> This patch is incremental to "cxl: Initialization and shutdown fixes" >>> >>> [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com >>> >>> drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- >>> 1 file changed, 33 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c >>> index 3478d2058303..dff618c708dc 100644 >>> --- a/drivers/cxl/core/region.c >>> +++ b/drivers/cxl/core/region.c > [..] >> >> Above checking seems meaningless since there are only one CLDX with ID >> port->commit_end + 1. >> >> logical of below my proposal maybe what you want. >> https://lore.kernel.org/all/4239bfd4-d5fe-4ac8-a087-9e1584765e61@icloud.com/ > > No. Consider that port->commit_end + 1 may walk off the end of available > decoders. really ? could you like to take a example ? let me paste the solution of below link here as well 1) it is simpler. 2) it does need to introduce a new API device_for_each_child_reverse_from() 3) it have the minimal iterating count. https://lore.kernel.org/all/4239bfd4-d5fe-4ac8-a087-9e1584765e61@icloud.com/ --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -796,8 +796,9 @@ 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_port *port = to_cxl_port(dev->parent); struct cxl_decoder *cxld; - int *id = data; + struct device **target_dev = data; if (!is_switch_decoder(dev)) return 0; @@ -805,15 +806,19 @@ static int match_free_decoder(struct device *dev, void *data) cxld = to_cxl_decoder(dev); /* enforce ordered allocation */ - if (cxld->id != *id) - return 0; - - if (!cxld->region) - return 1; - - (*id)++; - - return 0; + if (cxld->id == port->commit_end + 1) { + if (!cxld->region) { + *target_dev = dev; + return 1; + } else { + dev_dbg(dev->parent, + "next decoder to commit is already reserved\n", + dev_name(dev)); + return -ENODEV; + } + } else { + return cxld->flags & CXL_DECODER_F_ENABLE ? 0 : -EBUSY; + } } static int match_auto_decoder(struct device *dev, void *data) @@ -839,7 +844,7 @@ cxl_region_find_decoder(struct cxl_port *port, struct cxl_endpoint_decoder *cxled, struct cxl_region *cxlr) { - struct device *dev; + struct device *dev = NULL; int id = 0; if (port == cxled_to_port(cxled)) @@ -848,8 +853,8 @@ cxl_region_find_decoder(struct cxl_port *port, if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) dev = device_find_child(&port->dev, &cxlr->params, match_auto_decoder); - else - dev = device_find_child(&port->dev, &id, match_free_decoder); + else if (device_for_each_child(&port->dev, &dev, match_free_decoder) > 0) + get_device(dev); if (!dev) return NULL; /*
On 2024/10/15 08:06, Dan Williams wrote: > With the recent change to allow out-of-order decoder de-commit it > highlights a need to strengthen the in-order decoder commit guarantees. > As it stands match_free_decoder() ensures that if 2 regions are racing > decoder allocations the one that wins the race will get the lower id > decoder, but that still leaves the race to *commit* the decoder. > > Rather than have this complicated case of "reserved in-order, but may > still commit out-of-order", just arrange for the reservation order to > match the commit-order. In other words, prevent subsequent allocations > until the last reservation is committed. > > This precludes overlapping region creation events and requires the > previous regionN to either move forward to the decoder commit stage or > drop its reservation before regionN+1 can move forward. That is, > provided that regionN and regionN+1 decode through the same switch port. > > As a side effect this allows match_free_decoder() to drop its dependency > on needing write access to the device_find_child() @data parameter [1]. > > Reported-by: Zijun Hu <zijun_hu@icloud.com> > Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] Hi Dan, i also submitted v6 based on your recommendation but with some optimization here. https://lore.kernel.org/all/20241017-const_dfc_prepare-v6-1-fc842264a1cc@quicinc.com/ > Cc: Davidlohr Bueso <dave@stgolabs.net> > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Alison Schofield <alison.schofield@intel.com> > Cc: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > This patch is incremental to "cxl: Initialization and shutdown fixes" > > [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com > > drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 33 insertions(+), 10 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 3478d2058303..dff618c708dc 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -778,26 +778,50 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos) > return rc; > } > > +static int check_commit_order(struct device *dev, const void *data) > +{ > + struct cxl_decoder *cxld = to_cxl_decoder(dev); > + > + /* > + * if port->commit_end is not the only free decoder, then out of > + * order shutdown has occurred, block further allocations until > + * that is resolved > + */ > + if (((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) > + return -EBUSY; > + return 0; > +} > + > static int match_free_decoder(struct device *dev, void *data) > { > + struct cxl_port *port = to_cxl_port(dev->parent); > struct cxl_decoder *cxld; > - int *id = data; > + int rc; > > if (!is_switch_decoder(dev)) > return 0; > > cxld = to_cxl_decoder(dev); > > - /* enforce ordered allocation */ > - if (cxld->id != *id) > + if (cxld->id != port->commit_end + 1) > return 0; > > - if (!cxld->region) > - return 1; > - > - (*id)++; > + if (cxld->region) { > + dev_dbg(dev->parent, > + "next decoder to commit (%s) is already reserved (%s)\n", > + dev_name(dev), dev_name(&cxld->region->dev)); > + return 0; > + } > > - return 0; > + rc = device_for_each_child_reverse_from(dev->parent, dev, NULL, > + check_commit_order); > + if (rc) { > + dev_dbg(dev->parent, > + "unable to allocate %s due to out of order shutdown\n", > + dev_name(dev)); > + return 0; > + } > + return 1; > } > > static int match_auto_decoder(struct device *dev, void *data) > @@ -824,7 +848,6 @@ cxl_region_find_decoder(struct cxl_port *port, > struct cxl_region *cxlr) > { > struct device *dev; > - int id = 0; > > if (port == cxled_to_port(cxled)) > return &cxled->cxld; > @@ -833,7 +856,7 @@ 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); > + dev = device_find_child(&port->dev, NULL, match_free_decoder); > if (!dev) > return NULL; > /* >
On 2024/10/15 08:06, Dan Williams wrote: > With the recent change to allow out-of-order decoder de-commit it [snip] > > As a side effect this allows match_free_decoder() to drop its dependency > on needing write access to the device_find_child() @data parameter [1]. > Hi Dan Williams. is it possible to make its fix into in v6.13-rc1? i will start below bigger series which only depends on this fix ASAP. https://lore.kernel.org/all/20240811-const_dfc_done-v1-0-9d85e3f943cb@quicinc.com/ > Reported-by: Zijun Hu <zijun_hu@icloud.com> may use below one for v2 when possible Zijun Hu <quic_zijuhu@quicinc.com> > Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] > Cc: Davidlohr Bueso <dave@stgolabs.net> > Cc: Vishal Verma <vishal.l.verma@intel.com> > Cc: Alison Schofield <alison.schofield@intel.com> > Cc: Jonathan Cameron <jonathan.cameron@huawei.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > This patch is incremental to "cxl: Initialization and shutdown fixes" > > [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com > > drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 33 insertions(+), 10 deletions(-) > what about below optimized one? 1) it is simpler 2) it only uses exiting driver core API. 3) it has the minimal iterating count. you may use it as v2 if you also like it. https://patchwork.kernel.org/project/cxl/patch/20241017-const_dfc_prepare-v6-1-fc842264a1cc@quicinc.com/ +++ b/drivers/cxl/core/region.c @@ -796,8 +796,9 @@ 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_port *port = to_cxl_port(dev->parent); struct cxl_decoder *cxld; - int *id = data; + struct device **target_dev = data; if (!is_switch_decoder(dev)) return 0; @@ -805,15 +806,24 @@ static int match_free_decoder(struct device *dev, void *data) cxld = to_cxl_decoder(dev); /* enforce ordered allocation */ - if (cxld->id != *id) - return 0; + if (cxld->id == port->commit_end + 1) { + if (!cxld->region) { + *target_dev = dev; + return 1; + } - if (!cxld->region) - return 1; + dev_dbg(dev->parent, + "%s: cxld '%s' was reserved by '%s'\n", __func__, + dev_name(dev), dev_name(&cxld->region->dev)); + return -EBUSY; + } - (*id)++; + if (cxld->flags & CXL_DECODER_F_ENABLE) + return 0; - return 0; + dev_dbg(dev->parent, "%s: cxld '%s' was out of order shut down\n", + __func__, dev_name(dev)); + return -EINVAL; } static int match_auto_decoder(struct device *dev, void *data) @@ -839,17 +849,18 @@ 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; - if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) + if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) { dev = device_find_child(&port->dev, &cxlr->params, match_auto_decoder); - else - dev = device_find_child(&port->dev, &id, match_free_decoder); + } else { + device_for_each_child(&port->dev, &dev, match_free_decoder); + get_device(dev); + } if (!dev) return NULL; /* > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 3478d2058303..dff618c708dc 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -778,26 +778,50 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos) > return rc; > } > > +static int check_commit_order(struct device *dev, const void *data) > +{ > + struct cxl_decoder *cxld = to_cxl_decoder(dev); > + > + /* > + * if port->commit_end is not the only free decoder, then out of > + * order shutdown has occurred, block further allocations until > + * that is resolved > + */ > + if (((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) > + return -EBUSY; > + return 0; > +} > + > static int match_free_decoder(struct device *dev, void *data) > { > + struct cxl_port *port = to_cxl_port(dev->parent); > struct cxl_decoder *cxld; > - int *id = data; > + int rc; > > if (!is_switch_decoder(dev)) > return 0; > > cxld = to_cxl_decoder(dev); > > - /* enforce ordered allocation */ > - if (cxld->id != *id) > + if (cxld->id != port->commit_end + 1) > return 0; > > - if (!cxld->region) > - return 1; > - > - (*id)++; > + if (cxld->region) { > + dev_dbg(dev->parent, > + "next decoder to commit (%s) is already reserved (%s)\n", > + dev_name(dev), dev_name(&cxld->region->dev)); > + return 0; > + } > > - return 0; > + rc = device_for_each_child_reverse_from(dev->parent, dev, NULL, > + check_commit_order); > + if (rc) { > + dev_dbg(dev->parent, > + "unable to allocate %s due to out of order shutdown\n", > + dev_name(dev)); > + return 0; > + } > + return 1; > } > > static int match_auto_decoder(struct device *dev, void *data) > @@ -824,7 +848,6 @@ cxl_region_find_decoder(struct cxl_port *port, > struct cxl_region *cxlr) > { > struct device *dev; > - int id = 0; > > if (port == cxled_to_port(cxled)) > return &cxled->cxld; > @@ -833,7 +856,7 @@ 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); > + dev = device_find_child(&port->dev, NULL, match_free_decoder); > if (!dev) > return NULL; > /* >
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 3478d2058303..dff618c708dc 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -778,26 +778,50 @@ static size_t show_targetN(struct cxl_region *cxlr, char *buf, int pos) return rc; } +static int check_commit_order(struct device *dev, const void *data) +{ + struct cxl_decoder *cxld = to_cxl_decoder(dev); + + /* + * if port->commit_end is not the only free decoder, then out of + * order shutdown has occurred, block further allocations until + * that is resolved + */ + if (((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) + return -EBUSY; + return 0; +} + static int match_free_decoder(struct device *dev, void *data) { + struct cxl_port *port = to_cxl_port(dev->parent); struct cxl_decoder *cxld; - int *id = data; + int rc; if (!is_switch_decoder(dev)) return 0; cxld = to_cxl_decoder(dev); - /* enforce ordered allocation */ - if (cxld->id != *id) + if (cxld->id != port->commit_end + 1) return 0; - if (!cxld->region) - return 1; - - (*id)++; + if (cxld->region) { + dev_dbg(dev->parent, + "next decoder to commit (%s) is already reserved (%s)\n", + dev_name(dev), dev_name(&cxld->region->dev)); + return 0; + } - return 0; + rc = device_for_each_child_reverse_from(dev->parent, dev, NULL, + check_commit_order); + if (rc) { + dev_dbg(dev->parent, + "unable to allocate %s due to out of order shutdown\n", + dev_name(dev)); + return 0; + } + return 1; } static int match_auto_decoder(struct device *dev, void *data) @@ -824,7 +848,6 @@ cxl_region_find_decoder(struct cxl_port *port, struct cxl_region *cxlr) { struct device *dev; - int id = 0; if (port == cxled_to_port(cxled)) return &cxled->cxld; @@ -833,7 +856,7 @@ 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); + dev = device_find_child(&port->dev, NULL, match_free_decoder); if (!dev) return NULL; /*
With the recent change to allow out-of-order decoder de-commit it highlights a need to strengthen the in-order decoder commit guarantees. As it stands match_free_decoder() ensures that if 2 regions are racing decoder allocations the one that wins the race will get the lower id decoder, but that still leaves the race to *commit* the decoder. Rather than have this complicated case of "reserved in-order, but may still commit out-of-order", just arrange for the reservation order to match the commit-order. In other words, prevent subsequent allocations until the last reservation is committed. This precludes overlapping region creation events and requires the previous regionN to either move forward to the decoder commit stage or drop its reservation before regionN+1 can move forward. That is, provided that regionN and regionN+1 decode through the same switch port. As a side effect this allows match_free_decoder() to drop its dependency on needing write access to the device_find_child() @data parameter [1]. Reported-by: Zijun Hu <zijun_hu@icloud.com> Closes: http://lore.kernel.org/20240905-const_dfc_prepare-v4-0-4180e1d5a244@quicinc.com [1] Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: Vishal Verma <vishal.l.verma@intel.com> Cc: Alison Schofield <alison.schofield@intel.com> Cc: Jonathan Cameron <jonathan.cameron@huawei.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- This patch is incremental to "cxl: Initialization and shutdown fixes" [2]: http://lore.kernel.org/172862483180.2150669.5564474284074502692.stgit@dwillia2-xfh.jf.intel.com drivers/cxl/core/region.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-)