Linux CXL
 help / color / mirror / Atom feed
* [ndctl PATCH 0/2] daxctl, util/sysfs: fix builtin-driver false failure on enable
@ 2026-05-14  6:32 Chen Pei
  2026-05-14  6:32 ` [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure Chen Pei
  2026-05-14  6:32 ` [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live Chen Pei
  0 siblings, 2 replies; 8+ messages in thread
From: Chen Pei @ 2026-05-14  6:32 UTC (permalink / raw)
  To: alison.schofield, nvdimm; +Cc: linux-cxl, guoren, Chen Pei

When a DAX / ndctl driver is builtin (not a loadable module),
daxctl_insert_kmod_for_mode() and __util_bind() still call
kmod_module_probe_insert_module() unconditionally. libkmod only
short-circuits builtin modules when it can find the modules.builtin
index; otherwise it falls through to init_module() and returns -ENOENT,
surfacing as a spurious "insert failure".

Pre-check kmod_module_get_initstate() and skip probe-insert when the
module is already BUILTIN or LIVE, matching the pattern used by ndctl's
own test/core.c.

Chen Pei (2):
  daxctl: fix kmod reference leak on probe-insert failure
  daxctl, util/sysfs: skip module probe-insert when driver is builtin or
    live

 daxctl/lib/libdaxctl.c | 19 +++++++++++++++++--
 util/sysfs.c           | 17 +++++++++++------
 2 files changed, 28 insertions(+), 8 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure
  2026-05-14  6:32 [ndctl PATCH 0/2] daxctl, util/sysfs: fix builtin-driver false failure on enable Chen Pei
@ 2026-05-14  6:32 ` Chen Pei
  2026-05-14 17:16   ` Dave Jiang
  2026-05-14 22:14   ` Alison Schofield
  2026-05-14  6:32 ` [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live Chen Pei
  1 sibling, 2 replies; 8+ messages in thread
From: Chen Pei @ 2026-05-14  6:32 UTC (permalink / raw)
  To: alison.schofield, nvdimm; +Cc: linux-cxl, guoren, Chen Pei

daxctl_insert_kmod_for_mode() obtains a kmod reference via
kmod_module_new_from_name() and only stores it in dev->module after a
successful kmod_module_probe_insert_module() call. On the failure path
the local reference was returned without being released, leaking one
reference per failed enable attempt.

Drop the reference before returning the error code.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 daxctl/lib/libdaxctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index 02ae7e5..ffc81eb 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -927,6 +927,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
 			NULL, NULL, NULL, NULL);
 	if (rc < 0) {
 		err(ctx, "%s: insert failure: %d\n", devname, rc);
+		kmod_module_unref(kmod);
 		return rc;
 	}
 	dev->module = kmod;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live
  2026-05-14  6:32 [ndctl PATCH 0/2] daxctl, util/sysfs: fix builtin-driver false failure on enable Chen Pei
  2026-05-14  6:32 ` [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure Chen Pei
@ 2026-05-14  6:32 ` Chen Pei
  2026-05-14 17:17   ` Dave Jiang
  2026-05-14 18:37   ` Jonathan Cameron
  1 sibling, 2 replies; 8+ messages in thread
From: Chen Pei @ 2026-05-14  6:32 UTC (permalink / raw)
  To: alison.schofield, nvdimm; +Cc: linux-cxl, guoren, Chen Pei

kmod_module_probe_insert_module() is supposed to return 0 for builtin
modules, but only when libkmod can locate the modules.builtin index. If
the index is missing or out of sync, libkmod falls through to the real
init_module() syscall and returns an error such as -ENOENT, producing a
spurious "insert failure" even though the driver is already part of the
running kernel.

Pre-check kmod_module_get_initstate() and short-circuit when the module
is KMOD_MODULE_BUILTIN or KMOD_MODULE_LIVE, matching the pattern used by
ndctl's own test/core.c.

For builtin modules the local kmod reference is dropped because builtin
drivers cannot be unloaded; for live modules the reference is retained
in dev->module, matching the post-probe-success behavior.

Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
---
 daxctl/lib/libdaxctl.c | 18 ++++++++++++++++--
 util/sysfs.c           | 17 +++++++++++------
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
index ffc81eb..42bfc39 100644
--- a/daxctl/lib/libdaxctl.c
+++ b/daxctl/lib/libdaxctl.c
@@ -910,7 +910,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
 	const char *devname = daxctl_dev_get_devname(dev);
 	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
 	struct kmod_module *kmod;
-	int rc;
+	int state, rc;
 
 	rc = kmod_module_new_from_name(ctx->kmod_ctx, mod_name, &kmod);
 	if (rc < 0) {
@@ -919,7 +919,21 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
 		return rc;
 	}
 
-	/* if the driver is builtin, this Just Works */
+	/* If the driver is builtin or already live, skip probe-insert. */
+	state = kmod_module_get_initstate(kmod);
+	if (state == KMOD_MODULE_BUILTIN) {
+		dbg(ctx, "%s: module %s is builtin\n", devname,
+			kmod_module_get_name(kmod));
+		kmod_module_unref(kmod);
+		return 0;
+	}
+	if (state == KMOD_MODULE_LIVE) {
+		dbg(ctx, "%s: module %s already loaded\n", devname,
+			kmod_module_get_name(kmod));
+		dev->module = kmod;
+		return 0;
+	}
+
 	dbg(ctx, "%s inserting module: %s\n", devname,
 		kmod_module_get_name(kmod));
 	rc = kmod_module_probe_insert_module(kmod,
diff --git a/util/sysfs.c b/util/sysfs.c
index e027e38..641b86d 100644
--- a/util/sysfs.c
+++ b/util/sysfs.c
@@ -183,12 +183,17 @@ int __util_bind(const char *devname, struct kmod_module *module,
 	}
 
 	if (module) {
-		rc = kmod_module_probe_insert_module(module,
-						     KMOD_PROBE_APPLY_BLACKLIST,
-						     NULL, NULL, NULL, NULL);
-		if (rc < 0) {
-			log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
-			return rc;
+		/* Skip probe-insert when the module is already builtin or live. */
+		int state = kmod_module_get_initstate(module);
+
+		if (state != KMOD_MODULE_BUILTIN && state != KMOD_MODULE_LIVE) {
+			rc = kmod_module_probe_insert_module(module,
+							     KMOD_PROBE_APPLY_BLACKLIST,
+							     NULL, NULL, NULL, NULL);
+			if (rc < 0) {
+				log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
+				return rc;
+			}
 		}
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure
  2026-05-14  6:32 ` [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure Chen Pei
@ 2026-05-14 17:16   ` Dave Jiang
  2026-05-14 22:14   ` Alison Schofield
  1 sibling, 0 replies; 8+ messages in thread
From: Dave Jiang @ 2026-05-14 17:16 UTC (permalink / raw)
  To: Chen Pei, alison.schofield, nvdimm; +Cc: linux-cxl, guoren



On 5/13/26 11:32 PM, Chen Pei wrote:
> daxctl_insert_kmod_for_mode() obtains a kmod reference via
> kmod_module_new_from_name() and only stores it in dev->module after a
> successful kmod_module_probe_insert_module() call. On the failure path
> the local reference was returned without being released, leaking one
> reference per failed enable attempt.
> 
> Drop the reference before returning the error code.
> 
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>  daxctl/lib/libdaxctl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> index 02ae7e5..ffc81eb 100644
> --- a/daxctl/lib/libdaxctl.c
> +++ b/daxctl/lib/libdaxctl.c
> @@ -927,6 +927,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
>  			NULL, NULL, NULL, NULL);
>  	if (rc < 0) {
>  		err(ctx, "%s: insert failure: %d\n", devname, rc);
> +		kmod_module_unref(kmod);
>  		return rc;
>  	}
>  	dev->module = kmod;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live
  2026-05-14  6:32 ` [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live Chen Pei
@ 2026-05-14 17:17   ` Dave Jiang
  2026-05-14 18:37   ` Jonathan Cameron
  1 sibling, 0 replies; 8+ messages in thread
From: Dave Jiang @ 2026-05-14 17:17 UTC (permalink / raw)
  To: Chen Pei, alison.schofield, nvdimm; +Cc: linux-cxl, guoren



On 5/13/26 11:32 PM, Chen Pei wrote:
> kmod_module_probe_insert_module() is supposed to return 0 for builtin
> modules, but only when libkmod can locate the modules.builtin index. If
> the index is missing or out of sync, libkmod falls through to the real
> init_module() syscall and returns an error such as -ENOENT, producing a
> spurious "insert failure" even though the driver is already part of the
> running kernel.
> 
> Pre-check kmod_module_get_initstate() and short-circuit when the module
> is KMOD_MODULE_BUILTIN or KMOD_MODULE_LIVE, matching the pattern used by
> ndctl's own test/core.c.
> 
> For builtin modules the local kmod reference is dropped because builtin
> drivers cannot be unloaded; for live modules the reference is retained
> in dev->module, matching the post-probe-success behavior.
> 
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
>  daxctl/lib/libdaxctl.c | 18 ++++++++++++++++--
>  util/sysfs.c           | 17 +++++++++++------
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> index ffc81eb..42bfc39 100644
> --- a/daxctl/lib/libdaxctl.c
> +++ b/daxctl/lib/libdaxctl.c
> @@ -910,7 +910,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
>  	const char *devname = daxctl_dev_get_devname(dev);
>  	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
>  	struct kmod_module *kmod;
> -	int rc;
> +	int state, rc;
>  
>  	rc = kmod_module_new_from_name(ctx->kmod_ctx, mod_name, &kmod);
>  	if (rc < 0) {
> @@ -919,7 +919,21 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
>  		return rc;
>  	}
>  
> -	/* if the driver is builtin, this Just Works */
> +	/* If the driver is builtin or already live, skip probe-insert. */
> +	state = kmod_module_get_initstate(kmod);
> +	if (state == KMOD_MODULE_BUILTIN) {
> +		dbg(ctx, "%s: module %s is builtin\n", devname,
> +			kmod_module_get_name(kmod));
> +		kmod_module_unref(kmod);
> +		return 0;
> +	}
> +	if (state == KMOD_MODULE_LIVE) {
> +		dbg(ctx, "%s: module %s already loaded\n", devname,
> +			kmod_module_get_name(kmod));
> +		dev->module = kmod;
> +		return 0;
> +	}
> +
>  	dbg(ctx, "%s inserting module: %s\n", devname,
>  		kmod_module_get_name(kmod));
>  	rc = kmod_module_probe_insert_module(kmod,
> diff --git a/util/sysfs.c b/util/sysfs.c
> index e027e38..641b86d 100644
> --- a/util/sysfs.c
> +++ b/util/sysfs.c
> @@ -183,12 +183,17 @@ int __util_bind(const char *devname, struct kmod_module *module,
>  	}
>  
>  	if (module) {
> -		rc = kmod_module_probe_insert_module(module,
> -						     KMOD_PROBE_APPLY_BLACKLIST,
> -						     NULL, NULL, NULL, NULL);
> -		if (rc < 0) {
> -			log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> -			return rc;
> +		/* Skip probe-insert when the module is already builtin or live. */
> +		int state = kmod_module_get_initstate(module);
> +
> +		if (state != KMOD_MODULE_BUILTIN && state != KMOD_MODULE_LIVE) {
> +			rc = kmod_module_probe_insert_module(module,
> +							     KMOD_PROBE_APPLY_BLACKLIST,
> +							     NULL, NULL, NULL, NULL);
> +			if (rc < 0) {
> +				log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> +				return rc;
> +			}
>  		}
>  	}
>  


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live
  2026-05-14  6:32 ` [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live Chen Pei
  2026-05-14 17:17   ` Dave Jiang
@ 2026-05-14 18:37   ` Jonathan Cameron
  2026-05-14 22:13     ` Alison Schofield
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-14 18:37 UTC (permalink / raw)
  To: Chen Pei; +Cc: alison.schofield, nvdimm, linux-cxl, guoren

On Thu, 14 May 2026 14:32:34 +0800
Chen Pei <cp0613@linux.alibaba.com> wrote:

> kmod_module_probe_insert_module() is supposed to return 0 for builtin
> modules, but only when libkmod can locate the modules.builtin index. If
> the index is missing or out of sync, libkmod falls through to the real
> init_module() syscall and returns an error such as -ENOENT, producing a
> spurious "insert failure" even though the driver is already part of the
> running kernel.
> 
> Pre-check kmod_module_get_initstate() and short-circuit when the module
> is KMOD_MODULE_BUILTIN or KMOD_MODULE_LIVE, matching the pattern used by
> ndctl's own test/core.c.

So I happened to run into exactly this print earlier today and was
very happy to see this resolving it! I'm lazy so when developing in
a VM tend to do everything I care about built in and not bother with
installing the modules.

However - despite having CONFIG_DEV_DAX = y in the kernel, I'm getting
a state of KMOD_MODULE_COMING which is curious as there is no
initstate file to read that from.

Looking at the code in libkmod it seems to first check if it can open
/sys/modules/device_dax/initstate and if it can't checks if
the directory /sys/modules/device_dax/ exists. If it finds that it returns
KMOD_MODULE_COMING which seems odd given in a fully initialized built in driver
that particular set of circumstances is normal.

Any ideas?

To me the description above is misleading if we need to have something else
for the builtin case to work.

I'm out of time to today but may get time to look at this tomorrow and chase
down if there is a way to get it to work.

Jonathan


> 
> For builtin modules the local kmod reference is dropped because builtin
> drivers cannot be unloaded; for live modules the reference is retained
> in dev->module, matching the post-probe-success behavior.
> 
> Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
> ---
>  daxctl/lib/libdaxctl.c | 18 ++++++++++++++++--
>  util/sysfs.c           | 17 +++++++++++------
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> index ffc81eb..42bfc39 100644
> --- a/daxctl/lib/libdaxctl.c
> +++ b/daxctl/lib/libdaxctl.c
> @@ -910,7 +910,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
>  	const char *devname = daxctl_dev_get_devname(dev);
>  	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
>  	struct kmod_module *kmod;
> -	int rc;
> +	int state, rc;
>  
>  	rc = kmod_module_new_from_name(ctx->kmod_ctx, mod_name, &kmod);
>  	if (rc < 0) {
> @@ -919,7 +919,21 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
>  		return rc;
>  	}
>  
> -	/* if the driver is builtin, this Just Works */
> +	/* If the driver is builtin or already live, skip probe-insert. */
> +	state = kmod_module_get_initstate(kmod);
> +	if (state == KMOD_MODULE_BUILTIN) {
> +		dbg(ctx, "%s: module %s is builtin\n", devname,
> +			kmod_module_get_name(kmod));
> +		kmod_module_unref(kmod);
> +		return 0;
> +	}
> +	if (state == KMOD_MODULE_LIVE) {
> +		dbg(ctx, "%s: module %s already loaded\n", devname,
> +			kmod_module_get_name(kmod));
> +		dev->module = kmod;
> +		return 0;
> +	}
> +
>  	dbg(ctx, "%s inserting module: %s\n", devname,
>  		kmod_module_get_name(kmod));
>  	rc = kmod_module_probe_insert_module(kmod,
> diff --git a/util/sysfs.c b/util/sysfs.c
> index e027e38..641b86d 100644
> --- a/util/sysfs.c
> +++ b/util/sysfs.c
> @@ -183,12 +183,17 @@ int __util_bind(const char *devname, struct kmod_module *module,
>  	}
>  
>  	if (module) {
> -		rc = kmod_module_probe_insert_module(module,
> -						     KMOD_PROBE_APPLY_BLACKLIST,
> -						     NULL, NULL, NULL, NULL);
> -		if (rc < 0) {
> -			log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> -			return rc;
> +		/* Skip probe-insert when the module is already builtin or live. */
> +		int state = kmod_module_get_initstate(module);
> +
> +		if (state != KMOD_MODULE_BUILTIN && state != KMOD_MODULE_LIVE) {
> +			rc = kmod_module_probe_insert_module(module,
> +							     KMOD_PROBE_APPLY_BLACKLIST,
> +							     NULL, NULL, NULL, NULL);
> +			if (rc < 0) {
> +				log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> +				return rc;
> +			}
>  		}
>  	}
>  


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live
  2026-05-14 18:37   ` Jonathan Cameron
@ 2026-05-14 22:13     ` Alison Schofield
  0 siblings, 0 replies; 8+ messages in thread
From: Alison Schofield @ 2026-05-14 22:13 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Chen Pei, nvdimm, linux-cxl, guoren

On Thu, May 14, 2026 at 07:37:49PM +0100, Jonathan Cameron wrote:
> On Thu, 14 May 2026 14:32:34 +0800
> Chen Pei <cp0613@linux.alibaba.com> wrote:
> 
> > kmod_module_probe_insert_module() is supposed to return 0 for builtin
> > modules, but only when libkmod can locate the modules.builtin index. If
> > the index is missing or out of sync, libkmod falls through to the real
> > init_module() syscall and returns an error such as -ENOENT, producing a
> > spurious "insert failure" even though the driver is already part of the
> > running kernel.
> > 
> > Pre-check kmod_module_get_initstate() and short-circuit when the module
> > is KMOD_MODULE_BUILTIN or KMOD_MODULE_LIVE, matching the pattern used by
> > ndctl's own test/core.c.
> 
> So I happened to run into exactly this print earlier today and was
> very happy to see this resolving it! I'm lazy so when developing in
> a VM tend to do everything I care about built in and not bother with
> installing the modules.
> 
> However - despite having CONFIG_DEV_DAX = y in the kernel, I'm getting
> a state of KMOD_MODULE_COMING which is curious as there is no
> initstate file to read that from.

I think this patch is worth you trying. In libmkmod code I'm looking at:

https://github.com/lucasdemarchi/kmod/blob/master/libkmod/libkmod-module.c

the "module directory exists but initstate cannot be opened" case returns
KMOD_MODULE_BUILTIN, not KMOD_MODULE_COMING.

So if device_dax is builtin and /sys/module/device_dax exists without
initstate, this patch should short-circuit before attempting insert. If
you still see COMING with this patch applied, then we need to figure out
where that state is coming from (before thinking about special casing
it in ndctl).

> 
> Looking at the code in libkmod it seems to first check if it can open
> /sys/modules/device_dax/initstate and if it can't checks if
> the directory /sys/modules/device_dax/ exists. If it finds that it returns
> KMOD_MODULE_COMING which seems odd given in a fully initialized built in driver
> that particular set of circumstances is normal.
> 
> Any ideas?
> 
> To me the description above is misleading if we need to have something else
> for the builtin case to work.
> 
> I'm out of time to today but may get time to look at this tomorrow and chase
> down if there is a way to get it to work.
> 
> Jonathan
> 
> 
> > 
> > For builtin modules the local kmod reference is dropped because builtin
> > drivers cannot be unloaded; for live modules the reference is retained
> > in dev->module, matching the post-probe-success behavior.
> > 
> > Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
> > ---
> >  daxctl/lib/libdaxctl.c | 18 ++++++++++++++++--
> >  util/sysfs.c           | 17 +++++++++++------
> >  2 files changed, 27 insertions(+), 8 deletions(-)
> > 
> > diff --git a/daxctl/lib/libdaxctl.c b/daxctl/lib/libdaxctl.c
> > index ffc81eb..42bfc39 100644
> > --- a/daxctl/lib/libdaxctl.c
> > +++ b/daxctl/lib/libdaxctl.c
> > @@ -910,7 +910,7 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
> >  	const char *devname = daxctl_dev_get_devname(dev);
> >  	struct daxctl_ctx *ctx = daxctl_dev_get_ctx(dev);
> >  	struct kmod_module *kmod;
> > -	int rc;
> > +	int state, rc;
> >  
> >  	rc = kmod_module_new_from_name(ctx->kmod_ctx, mod_name, &kmod);
> >  	if (rc < 0) {
> > @@ -919,7 +919,21 @@ static int daxctl_insert_kmod_for_mode(struct daxctl_dev *dev,
> >  		return rc;
> >  	}
> >  
> > -	/* if the driver is builtin, this Just Works */
> > +	/* If the driver is builtin or already live, skip probe-insert. */
> > +	state = kmod_module_get_initstate(kmod);
> > +	if (state == KMOD_MODULE_BUILTIN) {
> > +		dbg(ctx, "%s: module %s is builtin\n", devname,
> > +			kmod_module_get_name(kmod));
> > +		kmod_module_unref(kmod);
> > +		return 0;
> > +	}
> > +	if (state == KMOD_MODULE_LIVE) {
> > +		dbg(ctx, "%s: module %s already loaded\n", devname,
> > +			kmod_module_get_name(kmod));
> > +		dev->module = kmod;
> > +		return 0;
> > +	}
> > +
> >  	dbg(ctx, "%s inserting module: %s\n", devname,
> >  		kmod_module_get_name(kmod));
> >  	rc = kmod_module_probe_insert_module(kmod,
> > diff --git a/util/sysfs.c b/util/sysfs.c
> > index e027e38..641b86d 100644
> > --- a/util/sysfs.c
> > +++ b/util/sysfs.c
> > @@ -183,12 +183,17 @@ int __util_bind(const char *devname, struct kmod_module *module,
> >  	}
> >  
> >  	if (module) {
> > -		rc = kmod_module_probe_insert_module(module,
> > -						     KMOD_PROBE_APPLY_BLACKLIST,
> > -						     NULL, NULL, NULL, NULL);
> > -		if (rc < 0) {
> > -			log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> > -			return rc;
> > +		/* Skip probe-insert when the module is already builtin or live. */
> > +		int state = kmod_module_get_initstate(module);
> > +
> > +		if (state != KMOD_MODULE_BUILTIN && state != KMOD_MODULE_LIVE) {
> > +			rc = kmod_module_probe_insert_module(module,
> > +							     KMOD_PROBE_APPLY_BLACKLIST,
> > +							     NULL, NULL, NULL, NULL);
> > +			if (rc < 0) {
> > +				log_err(ctx, "%s: insert failure: %d\n", __func__, rc);
> > +				return rc;
> > +			}
> >  		}
> >  	}
> >  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure
  2026-05-14  6:32 ` [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure Chen Pei
  2026-05-14 17:16   ` Dave Jiang
@ 2026-05-14 22:14   ` Alison Schofield
  1 sibling, 0 replies; 8+ messages in thread
From: Alison Schofield @ 2026-05-14 22:14 UTC (permalink / raw)
  To: Chen Pei; +Cc: nvdimm, linux-cxl, guoren

On Thu, May 14, 2026 at 02:32:33PM +0800, Chen Pei wrote:
> daxctl_insert_kmod_for_mode() obtains a kmod reference via
> kmod_module_new_from_name() and only stores it in dev->module after a
> successful kmod_module_probe_insert_module() call. On the failure path
> the local reference was returned without being released, leaking one
> reference per failed enable attempt.
> 
> Drop the reference before returning the error code.

Thanks!
Reviewed-by: Alison Schofield <alison.schofield@intel.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-14 22:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14  6:32 [ndctl PATCH 0/2] daxctl, util/sysfs: fix builtin-driver false failure on enable Chen Pei
2026-05-14  6:32 ` [ndctl PATCH 1/2] daxctl: fix kmod reference leak on probe-insert failure Chen Pei
2026-05-14 17:16   ` Dave Jiang
2026-05-14 22:14   ` Alison Schofield
2026-05-14  6:32 ` [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live Chen Pei
2026-05-14 17:17   ` Dave Jiang
2026-05-14 18:37   ` Jonathan Cameron
2026-05-14 22:13     ` Alison Schofield

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox