All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Chen Pei <cp0613@linux.alibaba.com>
Cc: alison.schofield@intel.com, nvdimm@lists.linux.dev,
	linux-cxl@vger.kernel.org, guoren@kernel.org
Subject: Re: [ndctl PATCH 2/2] daxctl, util/sysfs: skip module probe-insert when driver is builtin or live
Date: Thu, 14 May 2026 19:37:49 +0100	[thread overview]
Message-ID: <20260514193749.0f0750e2@jic23-huawei> (raw)
In-Reply-To: <20260514063234.86439-3-cp0613@linux.alibaba.com>

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;
> +			}
>  		}
>  	}
>  


  parent reply	other threads:[~2026-05-14 18:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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-15  2:27   ` Guo Ren
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 [this message]
2026-05-14 22:13     ` Alison Schofield

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260514193749.0f0750e2@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=alison.schofield@intel.com \
    --cc=cp0613@linux.alibaba.com \
    --cc=guoren@kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.