Re: [PATCH v3 0/2] cxl/region: Fix race conditions in cxl region unregistration.

Sungwoo Kim
2026-04-28T05:43:13Z

On Mon, Apr 27, 2026 at 1:42 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
>
>
> On 4/26/26 8:20 PM, Sungwoo Kim wrote:
> > This version mainly addresses Dave's comments and meaningful issues reported by Sashiko AI[1].
> >
> > Overview
> > ========
> > This patch series fixes race conditions in cxl region unregistration.
> >
> > devm_release_action() should be called once, otherwise, it warns about
> > the second call. However, the current implementation has a race condition
> > that allows multiple calls to devm_release_action(). The details are in
> > each patch.
> >
> > To fix these, the first patch adds a new function that guarantees that
> > devm_release_action() is called only once.
> > Using this function, the second patch subsitutes the current use of
> > devm_release_action() in cxl region with the new function.
> >
> > Change in v3
> > ============
> > Addressed Dave's comments:
> > - Split and made this in a patch series.
> > - Enhanced a commit log explaining why a workqueue is used in the first patch.
> > - Added a context on how these issues were found and what impact was observed and how that effects a user in the second patch.
> > Sashiko AI review fixes:
> > - Fixed construct_region() as it also can race.
> > - Used a driver's wq instead of system wq so unbinding can drain a work.
>
> A few issues Sashiko raised with the v3.
> https://sashiko.dev/#/patchset/20260427032010.916681-2-iam%40sung-woo.kim

A Sashiko's reasoning makes sense to me. In the middle of
construct_region(), users can perform a sysfs write, which then calls
unregister_region().
unregister_region() must not be called before construct_region() has
completed, since it calls device_del() and put_device().

This isn't introduced by this patch, but I can fix it in v4.

An enable/disable flag to allow sysfs write might fix this, like:

static struct cxl_region *construct_region(...)
{
... // when a construction is done
set_bit(CXL_REGION_F_ENABLE_SYSFS);
return cxlr;
}

This prevents a sysfs write before it's done:

static ssize_t delete_region_store(...)
{
if (!test_bit(CXL_REGION_F_ENABLE_SYSFS)) {
return -EBUSY;
}
...
}

Would there be a better solution? I'd like to ask for comments on this.
This patch series already introduces two new flags, and I'm concerned
about adding another.

Thank you!
Sungwoo.

>
> DJ
>
>
> >
> > [1] https://sashiko.dev/#/patchset/20260422045637.3048249-2-iam%40sung-woo.kim
> >
> > Earlier approach:
> > v2: https://lore.kernel.org/linux-cxl/20260422045637.3048249-2-iam@sung-woo.kim/
> > v1: https://lore.kernel.org/linux-cxl/20260308185958.2453707-2-iam@sung-woo.kim/
> >
> > Sungwoo Kim (2):
> > cxl/region: serialize devm action removal via scheduled work
> > cxl/region: Fix a race bug in delete_region_store()
> >
> > drivers/cxl/core/port.c | 6 +++++
> > drivers/cxl/core/region.c | 47 ++++++++++++++++++++++++++++++++++-----
> > drivers/cxl/cxl.h | 9 ++++++++
> > 3 files changed, 57 insertions(+), 5 deletions(-)
> >
>
>