| Message ID | 2aa10593dbe6a4a8da35077aa492c8aacd363901.1691176651.git.alison.schofield@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/region: Improve Soft Reserved resource handling | expand |
On 8/4/23 12:31, alison.schofield@intel.com wrote: > From: Alison Schofield <alison.schofield@intel.com> > > When CXL regions are created through autodiscovery their resource > may be a child of a soft reserved resource. Today, when such a > region is destroyed, its soft reserved resource remains in place. > That means that the HPA range that the region could release, is > actually unavailable for reuse. I think it reads better without the comma here. > > Free the soft reserved resource on region teardown by examining > the alignment of the resources, and handling accordingly. The > two resources, will be exactly aligned, partially aligned, or not > aligned at all. > > |----------- "Soft Reserved" -----------| > |-------------- "Region #" -------------| > Exactly aligned. Any dangling children move up to a parent on removal. > The removal of this soft reserved seems guaranteed, however, the > availability of the address range for use depends on complete cleanup > of the region child resources also. > > |----------- "Soft Reserved" -----------| > |-------- "Region #" -------| > or > |----------- "Soft Reserved" -----------| > |-------- "Region #" -------| > Either start or end aligns. Unlike removing a resource, which simply > moves child resources up the resource tree, adjustments fail if any > child resources map the range being truncated. So, this one will fail > for dangling child resources of the region. > > |---------- "Soft Reserved" ----------| > |---- "Region #" ----| > No alignment. Freeing the resource of a region in the middle is possible > if no child resources map the leading or trailing address space. > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > --- > drivers/cxl/core/region.c | 132 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 123 insertions(+), 9 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 36c0c0dd5697..b8a02afa3514 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -569,22 +569,136 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > return 0; > } > > +static int add_soft_reserved(struct resource *parent, resource_size_t start, > + resource_size_t len, unsigned long flags) > +{ > + struct resource *res; > + int rc; > + > + res = kmalloc(sizeof(*res), GFP_KERNEL); > + if (!res) > + return -ENOMEM; > + > + *res = DEFINE_RES_MEM_NAMED(start, len, "Soft Reserved"); > + > + res->desc = IORES_DESC_SOFT_RESERVED; > + res->flags = res->flags | flags; > + rc = insert_resource(parent, res); > + if (rc) > + kfree(res); > + > + return rc; > +} > + > +static void remove_soft_reserved(struct cxl_region *cxlr, struct resource *soft, > + resource_size_t start, resource_size_t end) > +{ > + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); > + resource_size_t new_start, new_end; > + struct resource *parent; > + unsigned long flags; > + int rc; > + > + /* Prevent new usage while removing or adjusting the resource */ > + mutex_lock(&cxlrd->range_lock); > + > + /* Aligns at both resource start and end */ > + if (soft->start == start && soft->end == end) { > + rc = remove_resource(soft); > + if (rc) > + dev_dbg(&cxlr->dev, > + "cannot remove soft reserved resource %pr\n", > + soft); > + else > + kfree(soft); > + > + goto out; > + } > + > + /* Aligns at either resource start or end */ > + if (soft->start == start || soft->end == end) { > + if (soft->start == start) { > + new_start = end + 1; > + new_end = soft->end; > + } else { > + new_start = soft->start; > + new_end = start + 1; > + } > + rc = adjust_resource(soft, new_start, new_end - new_start + 1); > + if (rc) > + dev_dbg(&cxlr->dev, > + "cannot adjust soft reserved resource %pr\n", > + soft); > + goto out; > + } > + > + /* > + * No alignment. Attempt a 3-way split that removes the part of > + * the resource the region occupied, and then creates new soft > + * reserved resources for the leading and trailing addr space. > + * adjust_resource() will stop the attempt if there are any > + * child resources. > + */ > + > + /* Save the original soft reserved resource params before adjusting */ > + new_start = soft->start; > + new_end = soft->end; > + parent = soft->parent; > + flags = soft->flags; > + > + rc = adjust_resource(soft, start, end - start); > + if (rc) { > + dev_dbg(&cxlr->dev, > + "cannot adjust soft reserved resource %pr\n", soft); > + goto out; > + } > + rc = remove_resource(soft); > + if (rc) > + dev_warn(&cxlr->dev, > + "cannot remove soft reserved resource %pr\n", soft); > + > + rc = add_soft_reserved(parent, new_start, start - new_start, flags); > + if (rc) > + dev_warn(&cxlr->dev, > + "cannot add new soft reserved resource at %pa\n", > + &new_start); > + > + rc = add_soft_reserved(parent, end + 1, new_end - end, flags); > + if (rc) > + dev_warn(&cxlr->dev, > + "cannot add new soft reserved resource at %pa + 1\n", > + &end); > +out: > + mutex_unlock(&cxlrd->range_lock); > +} > + > static void cxl_region_iomem_release(struct cxl_region *cxlr) > { > struct cxl_region_params *p = &cxlr->params; > + struct resource *parent, *res = p->res; > + resource_size_t start, end; > > if (device_is_registered(&cxlr->dev)) > lockdep_assert_held_write(&cxl_region_rwsem); > - if (p->res) { > - /* > - * Autodiscovered regions may not have been able to insert their > - * resource. > - */ > - if (p->res->parent) > - remove_resource(p->res); > - kfree(p->res); > - p->res = NULL; > + if (!res) > + return; > + /* > + * Autodiscovered regions may not have been able to insert their > + * resource. If a Soft Reserved parent resource exists, try to > + * remove that also. > + */ > + if (p->res->parent) { > + parent = p->res->parent; > + start = p->res->start; > + end = p->res->end; > + remove_resource(p->res); > + if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags) && > + parent->desc == IORES_DESC_SOFT_RESERVED) > + remove_soft_reserved(cxlr, parent, start, end); > } > + > + kfree(p->res); > + p->res = NULL; > } > > static int free_hpa(struct cxl_region *cxlr)
On Wed, Aug 09, 2023 at 04:34:43PM -0700, Dave Jiang wrote: > > > On 8/4/23 12:31, alison.schofield@intel.com wrote: > > From: Alison Schofield <alison.schofield@intel.com> > > > > When CXL regions are created through autodiscovery their resource > > may be a child of a soft reserved resource. Today, when such a > > region is destroyed, its soft reserved resource remains in place. > > That means that the HPA range that the region could release, is > > actually unavailable for reuse. > > I think it reads better without the comma here. Agree. Thanks! Alison > > > > > Free the soft reserved resource on region teardown by examining > > the alignment of the resources, and handling accordingly. The > > two resources, will be exactly aligned, partially aligned, or not > > aligned at all. > > > > |----------- "Soft Reserved" -----------| > > |-------------- "Region #" -------------| > > Exactly aligned. Any dangling children move up to a parent on removal. > > The removal of this soft reserved seems guaranteed, however, the > > availability of the address range for use depends on complete cleanup > > of the region child resources also. > > > > |----------- "Soft Reserved" -----------| > > |-------- "Region #" -------| > > or > > |----------- "Soft Reserved" -----------| > > |-------- "Region #" -------| > > Either start or end aligns. Unlike removing a resource, which simply > > moves child resources up the resource tree, adjustments fail if any > > child resources map the range being truncated. So, this one will fail > > for dangling child resources of the region. > > > > |---------- "Soft Reserved" ----------| > > |---- "Region #" ----| > > No alignment. Freeing the resource of a region in the middle is possible > > if no child resources map the leading or trailing address space. > > > > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > > --- > > drivers/cxl/core/region.c | 132 +++++++++++++++++++++++++++++++++++--- > > 1 file changed, 123 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index 36c0c0dd5697..b8a02afa3514 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > @@ -569,22 +569,136 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > > return 0; > > } > > +static int add_soft_reserved(struct resource *parent, resource_size_t start, > > + resource_size_t len, unsigned long flags) > > +{ > > + struct resource *res; > > + int rc; > > + > > + res = kmalloc(sizeof(*res), GFP_KERNEL); > > + if (!res) > > + return -ENOMEM; > > + > > + *res = DEFINE_RES_MEM_NAMED(start, len, "Soft Reserved"); > > + > > + res->desc = IORES_DESC_SOFT_RESERVED; > > + res->flags = res->flags | flags; > > + rc = insert_resource(parent, res); > > + if (rc) > > + kfree(res); > > + > > + return rc; > > +} > > + > > +static void remove_soft_reserved(struct cxl_region *cxlr, struct resource *soft, > > + resource_size_t start, resource_size_t end) > > +{ > > + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); > > + resource_size_t new_start, new_end; > > + struct resource *parent; > > + unsigned long flags; > > + int rc; > > + > > + /* Prevent new usage while removing or adjusting the resource */ > > + mutex_lock(&cxlrd->range_lock); > > + > > + /* Aligns at both resource start and end */ > > + if (soft->start == start && soft->end == end) { > > + rc = remove_resource(soft); > > + if (rc) > > + dev_dbg(&cxlr->dev, > > + "cannot remove soft reserved resource %pr\n", > > + soft); > > + else > > + kfree(soft); > > + > > + goto out; > > + } > > + > > + /* Aligns at either resource start or end */ > > + if (soft->start == start || soft->end == end) { > > + if (soft->start == start) { > > + new_start = end + 1; > > + new_end = soft->end; > > + } else { > > + new_start = soft->start; > > + new_end = start + 1; > > + } > > + rc = adjust_resource(soft, new_start, new_end - new_start + 1); > > + if (rc) > > + dev_dbg(&cxlr->dev, > > + "cannot adjust soft reserved resource %pr\n", > > + soft); > > + goto out; > > + } > > + > > + /* > > + * No alignment. Attempt a 3-way split that removes the part of > > + * the resource the region occupied, and then creates new soft > > + * reserved resources for the leading and trailing addr space. > > + * adjust_resource() will stop the attempt if there are any > > + * child resources. > > + */ > > + > > + /* Save the original soft reserved resource params before adjusting */ > > + new_start = soft->start; > > + new_end = soft->end; > > + parent = soft->parent; > > + flags = soft->flags; > > + > > + rc = adjust_resource(soft, start, end - start); > > + if (rc) { > > + dev_dbg(&cxlr->dev, > > + "cannot adjust soft reserved resource %pr\n", soft); > > + goto out; > > + } > > + rc = remove_resource(soft); > > + if (rc) > > + dev_warn(&cxlr->dev, > > + "cannot remove soft reserved resource %pr\n", soft); > > + > > + rc = add_soft_reserved(parent, new_start, start - new_start, flags); > > + if (rc) > > + dev_warn(&cxlr->dev, > > + "cannot add new soft reserved resource at %pa\n", > > + &new_start); > > + > > + rc = add_soft_reserved(parent, end + 1, new_end - end, flags); > > + if (rc) > > + dev_warn(&cxlr->dev, > > + "cannot add new soft reserved resource at %pa + 1\n", > > + &end); > > +out: > > + mutex_unlock(&cxlrd->range_lock); > > +} > > + > > static void cxl_region_iomem_release(struct cxl_region *cxlr) > > { > > struct cxl_region_params *p = &cxlr->params; > > + struct resource *parent, *res = p->res; > > + resource_size_t start, end; > > if (device_is_registered(&cxlr->dev)) > > lockdep_assert_held_write(&cxl_region_rwsem); > > - if (p->res) { > > - /* > > - * Autodiscovered regions may not have been able to insert their > > - * resource. > > - */ > > - if (p->res->parent) > > - remove_resource(p->res); > > - kfree(p->res); > > - p->res = NULL; > > + if (!res) > > + return; > > + /* > > + * Autodiscovered regions may not have been able to insert their > > + * resource. If a Soft Reserved parent resource exists, try to > > + * remove that also. > > + */ > > + if (p->res->parent) { > > + parent = p->res->parent; > > + start = p->res->start; > > + end = p->res->end; > > + remove_resource(p->res); > > + if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags) && > > + parent->desc == IORES_DESC_SOFT_RESERVED) > > + remove_soft_reserved(cxlr, parent, start, end); > > } > > + > > + kfree(p->res); > > + p->res = NULL; > > } > > static int free_hpa(struct cxl_region *cxlr)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 36c0c0dd5697..b8a02afa3514 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -569,22 +569,136 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) return 0; } +static int add_soft_reserved(struct resource *parent, resource_size_t start, + resource_size_t len, unsigned long flags) +{ + struct resource *res; + int rc; + + res = kmalloc(sizeof(*res), GFP_KERNEL); + if (!res) + return -ENOMEM; + + *res = DEFINE_RES_MEM_NAMED(start, len, "Soft Reserved"); + + res->desc = IORES_DESC_SOFT_RESERVED; + res->flags = res->flags | flags; + rc = insert_resource(parent, res); + if (rc) + kfree(res); + + return rc; +} + +static void remove_soft_reserved(struct cxl_region *cxlr, struct resource *soft, + resource_size_t start, resource_size_t end) +{ + struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent); + resource_size_t new_start, new_end; + struct resource *parent; + unsigned long flags; + int rc; + + /* Prevent new usage while removing or adjusting the resource */ + mutex_lock(&cxlrd->range_lock); + + /* Aligns at both resource start and end */ + if (soft->start == start && soft->end == end) { + rc = remove_resource(soft); + if (rc) + dev_dbg(&cxlr->dev, + "cannot remove soft reserved resource %pr\n", + soft); + else + kfree(soft); + + goto out; + } + + /* Aligns at either resource start or end */ + if (soft->start == start || soft->end == end) { + if (soft->start == start) { + new_start = end + 1; + new_end = soft->end; + } else { + new_start = soft->start; + new_end = start + 1; + } + rc = adjust_resource(soft, new_start, new_end - new_start + 1); + if (rc) + dev_dbg(&cxlr->dev, + "cannot adjust soft reserved resource %pr\n", + soft); + goto out; + } + + /* + * No alignment. Attempt a 3-way split that removes the part of + * the resource the region occupied, and then creates new soft + * reserved resources for the leading and trailing addr space. + * adjust_resource() will stop the attempt if there are any + * child resources. + */ + + /* Save the original soft reserved resource params before adjusting */ + new_start = soft->start; + new_end = soft->end; + parent = soft->parent; + flags = soft->flags; + + rc = adjust_resource(soft, start, end - start); + if (rc) { + dev_dbg(&cxlr->dev, + "cannot adjust soft reserved resource %pr\n", soft); + goto out; + } + rc = remove_resource(soft); + if (rc) + dev_warn(&cxlr->dev, + "cannot remove soft reserved resource %pr\n", soft); + + rc = add_soft_reserved(parent, new_start, start - new_start, flags); + if (rc) + dev_warn(&cxlr->dev, + "cannot add new soft reserved resource at %pa\n", + &new_start); + + rc = add_soft_reserved(parent, end + 1, new_end - end, flags); + if (rc) + dev_warn(&cxlr->dev, + "cannot add new soft reserved resource at %pa + 1\n", + &end); +out: + mutex_unlock(&cxlrd->range_lock); +} + static void cxl_region_iomem_release(struct cxl_region *cxlr) { struct cxl_region_params *p = &cxlr->params; + struct resource *parent, *res = p->res; + resource_size_t start, end; if (device_is_registered(&cxlr->dev)) lockdep_assert_held_write(&cxl_region_rwsem); - if (p->res) { - /* - * Autodiscovered regions may not have been able to insert their - * resource. - */ - if (p->res->parent) - remove_resource(p->res); - kfree(p->res); - p->res = NULL; + if (!res) + return; + /* + * Autodiscovered regions may not have been able to insert their + * resource. If a Soft Reserved parent resource exists, try to + * remove that also. + */ + if (p->res->parent) { + parent = p->res->parent; + start = p->res->start; + end = p->res->end; + remove_resource(p->res); + if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags) && + parent->desc == IORES_DESC_SOFT_RESERVED) + remove_soft_reserved(cxlr, parent, start, end); } + + kfree(p->res); + p->res = NULL; } static int free_hpa(struct cxl_region *cxlr)