* [PATCH v3] ipmi:si: Add async init to ipmi_si
@ 2026-08-10 7:48 Michal Clapinski
2026-08-12 21:29 ` Corey Minyard
0 siblings, 1 reply; 2+ messages in thread
From: Michal Clapinski @ 2026-08-10 7:48 UTC (permalink / raw)
To: Corey Minyard, openipmi-developer; +Cc: linux-kernel, Michal Clapinski
Added a new config option to allow offloading individual calls to
try_smi_init() using workqueue.
Saves 100ms on my system.
Signed-off-by: Michal Clapinski <mclapinski@google.com>
---
v3:
- removed __init from the async function
- reimplemented the whole thing with a workqueue
- added cancel_work_sync to cleanup_one_si, which means cleanup_one_si
now has to run without the smi_infos_lock
v2:
- instead of offloading the whole init function, offload just the
individual calls to try_smi_init()
---
drivers/char/ipmi/Kconfig | 9 ++++
drivers/char/ipmi/ipmi_si_intf.c | 79 +++++++++++++++++++++++++-------
2 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index 669f76000197..538a7d3c65bf 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -67,6 +67,15 @@ config IPMI_SI
Currently, only KCS and SMIC are supported. If
you are using IPMI, you should probably say "y" here.
+config IPMI_SI_ASYNC_INIT
+ bool 'Asynchronous initialization of IPMI System Interface'
+ depends on IPMI_SI
+ default n
+ help
+ Offloads individual SMI inits. It speeds up the boot time.
+ It also introduces a very small risk that something else might fail
+ if it depends on synchronous IPMI init.
+
config IPMI_SSIF
tristate 'IPMI SMBus handler (SSIF)'
depends on I2C
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 9a9d12be9bf7..79c510a8d00a 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -39,6 +39,7 @@
#include <linux/rcupdate.h>
#include <linux/ipmi.h>
#include <linux/ipmi_smi.h>
+#include <linux/workqueue.h>
#include "ipmi_si.h"
#include "ipmi_si_sm.h"
#include <linux/string.h>
@@ -252,6 +253,8 @@ struct smi_info {
struct task_struct *thread;
+ struct work_struct init_work;
+
struct list_head link;
};
@@ -272,6 +275,7 @@ static bool unload_when_empty = true;
static int try_smi_init(struct smi_info *smi);
static void cleanup_one_si(struct smi_info *smi_info);
static void cleanup_ipmi_si(void);
+static void smi_init_work_fn(struct work_struct *work);
#ifdef DEBUG_TIMING
void debug_timestamp(struct smi_info *smi_info, char *msg)
@@ -1970,6 +1974,7 @@ int ipmi_si_add_smi(struct si_sm_io *io)
if (!new_smi)
return -ENOMEM;
spin_lock_init(&new_smi->si_lock);
+ INIT_WORK(&new_smi->init_work, smi_init_work_fn);
new_smi->io = *io;
@@ -1982,7 +1987,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
dev_info(dup->io.dev,
"Removing SMBIOS-specified %s state machine in favor of ACPI\n",
si_to_str[new_smi->io.si_info->type]);
+ list_del(&dup->link);
+ mutex_unlock(&smi_infos_lock);
+
cleanup_one_si(dup);
+
+ mutex_lock(&smi_infos_lock);
} else {
dev_info(new_smi->io.dev,
"%s-specified %s state machine: duplicate\n",
@@ -2000,8 +2010,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
list_add_tail(&new_smi->link, &smi_infos);
- if (initialized)
- rv = try_smi_init(new_smi);
+ if (initialized) {
+ if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+ queue_work(system_unbound_wq, &new_smi->init_work);
+ else
+ rv = try_smi_init(new_smi);
+ }
out_err:
mutex_unlock(&smi_infos_lock);
return rv;
@@ -2174,6 +2188,15 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
e1->io.addr_data == e2->io.addr_data);
}
+static void smi_init_work_fn(struct work_struct *work)
+{
+ struct smi_info *smi = container_of(work, struct smi_info, init_work);
+
+ mutex_lock(&smi_infos_lock);
+ try_smi_init(smi);
+ mutex_unlock(&smi_infos_lock);
+}
+
static int __init init_ipmi_si(void)
{
struct smi_info *e, *e2;
@@ -2219,8 +2242,12 @@ static int __init init_ipmi_si(void)
break;
}
}
- if (!dup)
- try_smi_init(e);
+ if (!dup) {
+ if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+ queue_work(system_unbound_wq, &e->init_work);
+ else
+ try_smi_init(e);
+ }
}
/*
@@ -2253,8 +2280,12 @@ static int __init init_ipmi_si(void)
break;
}
}
- if (!dup)
- try_smi_init(e);
+ if (!dup) {
+ if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+ queue_work(system_unbound_wq, &e->init_work);
+ else
+ try_smi_init(e);
+ }
}
initialized = true;
@@ -2344,31 +2375,36 @@ static void shutdown_smi(void *send_info)
}
/*
- * Must be called with smi_infos_lock held, to serialize the
- * smi_info->intf check.
+ * Must be called with smi_info unlinked from smi_infos and smi_infos_lock released.
*/
static void cleanup_one_si(struct smi_info *smi_info)
{
if (!smi_info)
return;
- list_del(&smi_info->link);
+ if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+ cancel_work_sync(&smi_info->init_work);
+
ipmi_unregister_smi(smi_info->intf);
kfree(smi_info);
}
void ipmi_si_remove_by_dev(struct device *dev)
{
- struct smi_info *e;
+ struct smi_info *e = NULL, *tmp;
mutex_lock(&smi_infos_lock);
- list_for_each_entry(e, &smi_infos, link) {
- if (e->io.dev == dev) {
- cleanup_one_si(e);
+ list_for_each_entry(tmp, &smi_infos, link) {
+ if (tmp->io.dev == dev) {
+ e = tmp;
+ list_del(&e->link);
break;
}
}
mutex_unlock(&smi_infos_lock);
+
+ if (e)
+ cleanup_one_si(e);
}
struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
@@ -2377,6 +2413,7 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
/* remove */
struct smi_info *e, *tmp_e;
struct device *dev = NULL;
+ LIST_HEAD(to_clean);
mutex_lock(&smi_infos_lock);
list_for_each_entry_safe(e, tmp_e, &smi_infos, link) {
@@ -2386,17 +2423,23 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
continue;
if (e->io.addr_data == addr) {
dev = get_device(e->io.dev);
- cleanup_one_si(e);
+ list_move_tail(&e->link, &to_clean);
}
}
mutex_unlock(&smi_infos_lock);
+ list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
+ list_del(&e->link);
+ cleanup_one_si(e);
+ }
+
return dev;
}
static void cleanup_ipmi_si(void)
{
struct smi_info *e, *tmp_e;
+ LIST_HEAD(to_clean);
if (!initialized)
return;
@@ -2410,10 +2453,14 @@ static void cleanup_ipmi_si(void)
ipmi_si_platform_shutdown();
mutex_lock(&smi_infos_lock);
- list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
- cleanup_one_si(e);
+ list_splice_init(&smi_infos, &to_clean);
mutex_unlock(&smi_infos_lock);
+ list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
+ list_del(&e->link);
+ cleanup_one_si(e);
+ }
+
ipmi_si_hardcode_exit();
ipmi_si_hotmod_exit();
}
--
2.55.0.654.g21b8a5bc05-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] ipmi:si: Add async init to ipmi_si
2026-08-10 7:48 [PATCH v3] ipmi:si: Add async init to ipmi_si Michal Clapinski
@ 2026-08-12 21:29 ` Corey Minyard
0 siblings, 0 replies; 2+ messages in thread
From: Corey Minyard @ 2026-08-12 21:29 UTC (permalink / raw)
To: Michal Clapinski; +Cc: openipmi-developer, linux-kernel
On Mon, Aug 10, 2026 at 09:48:51AM +0200, Michal Clapinski wrote:
> Added a new config option to allow offloading individual calls to
> try_smi_init() using workqueue.
>
> Saves 100ms on my system.
Looks like you covered all the bases for this.
It's in my linux-next tree. I'll be running test suites on it in the
near future.
-corey
>
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
> v3:
> - removed __init from the async function
> - reimplemented the whole thing with a workqueue
> - added cancel_work_sync to cleanup_one_si, which means cleanup_one_si
> now has to run without the smi_infos_lock
> v2:
> - instead of offloading the whole init function, offload just the
> individual calls to try_smi_init()
> ---
> drivers/char/ipmi/Kconfig | 9 ++++
> drivers/char/ipmi/ipmi_si_intf.c | 79 +++++++++++++++++++++++++-------
> 2 files changed, 72 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 669f76000197..538a7d3c65bf 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -67,6 +67,15 @@ config IPMI_SI
> Currently, only KCS and SMIC are supported. If
> you are using IPMI, you should probably say "y" here.
>
> +config IPMI_SI_ASYNC_INIT
> + bool 'Asynchronous initialization of IPMI System Interface'
> + depends on IPMI_SI
> + default n
> + help
> + Offloads individual SMI inits. It speeds up the boot time.
> + It also introduces a very small risk that something else might fail
> + if it depends on synchronous IPMI init.
> +
> config IPMI_SSIF
> tristate 'IPMI SMBus handler (SSIF)'
> depends on I2C
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 9a9d12be9bf7..79c510a8d00a 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -39,6 +39,7 @@
> #include <linux/rcupdate.h>
> #include <linux/ipmi.h>
> #include <linux/ipmi_smi.h>
> +#include <linux/workqueue.h>
> #include "ipmi_si.h"
> #include "ipmi_si_sm.h"
> #include <linux/string.h>
> @@ -252,6 +253,8 @@ struct smi_info {
>
> struct task_struct *thread;
>
> + struct work_struct init_work;
> +
> struct list_head link;
> };
>
> @@ -272,6 +275,7 @@ static bool unload_when_empty = true;
> static int try_smi_init(struct smi_info *smi);
> static void cleanup_one_si(struct smi_info *smi_info);
> static void cleanup_ipmi_si(void);
> +static void smi_init_work_fn(struct work_struct *work);
>
> #ifdef DEBUG_TIMING
> void debug_timestamp(struct smi_info *smi_info, char *msg)
> @@ -1970,6 +1974,7 @@ int ipmi_si_add_smi(struct si_sm_io *io)
> if (!new_smi)
> return -ENOMEM;
> spin_lock_init(&new_smi->si_lock);
> + INIT_WORK(&new_smi->init_work, smi_init_work_fn);
>
> new_smi->io = *io;
>
> @@ -1982,7 +1987,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
> dev_info(dup->io.dev,
> "Removing SMBIOS-specified %s state machine in favor of ACPI\n",
> si_to_str[new_smi->io.si_info->type]);
> + list_del(&dup->link);
> + mutex_unlock(&smi_infos_lock);
> +
> cleanup_one_si(dup);
> +
> + mutex_lock(&smi_infos_lock);
> } else {
> dev_info(new_smi->io.dev,
> "%s-specified %s state machine: duplicate\n",
> @@ -2000,8 +2010,12 @@ int ipmi_si_add_smi(struct si_sm_io *io)
>
> list_add_tail(&new_smi->link, &smi_infos);
>
> - if (initialized)
> - rv = try_smi_init(new_smi);
> + if (initialized) {
> + if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> + queue_work(system_unbound_wq, &new_smi->init_work);
> + else
> + rv = try_smi_init(new_smi);
> + }
> out_err:
> mutex_unlock(&smi_infos_lock);
> return rv;
> @@ -2174,6 +2188,15 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
> e1->io.addr_data == e2->io.addr_data);
> }
>
> +static void smi_init_work_fn(struct work_struct *work)
> +{
> + struct smi_info *smi = container_of(work, struct smi_info, init_work);
> +
> + mutex_lock(&smi_infos_lock);
> + try_smi_init(smi);
> + mutex_unlock(&smi_infos_lock);
> +}
> +
> static int __init init_ipmi_si(void)
> {
> struct smi_info *e, *e2;
> @@ -2219,8 +2242,12 @@ static int __init init_ipmi_si(void)
> break;
> }
> }
> - if (!dup)
> - try_smi_init(e);
> + if (!dup) {
> + if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> + queue_work(system_unbound_wq, &e->init_work);
> + else
> + try_smi_init(e);
> + }
> }
>
> /*
> @@ -2253,8 +2280,12 @@ static int __init init_ipmi_si(void)
> break;
> }
> }
> - if (!dup)
> - try_smi_init(e);
> + if (!dup) {
> + if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> + queue_work(system_unbound_wq, &e->init_work);
> + else
> + try_smi_init(e);
> + }
> }
>
> initialized = true;
> @@ -2344,31 +2375,36 @@ static void shutdown_smi(void *send_info)
> }
>
> /*
> - * Must be called with smi_infos_lock held, to serialize the
> - * smi_info->intf check.
> + * Must be called with smi_info unlinked from smi_infos and smi_infos_lock released.
> */
> static void cleanup_one_si(struct smi_info *smi_info)
> {
> if (!smi_info)
> return;
>
> - list_del(&smi_info->link);
> + if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> + cancel_work_sync(&smi_info->init_work);
> +
> ipmi_unregister_smi(smi_info->intf);
> kfree(smi_info);
> }
>
> void ipmi_si_remove_by_dev(struct device *dev)
> {
> - struct smi_info *e;
> + struct smi_info *e = NULL, *tmp;
>
> mutex_lock(&smi_infos_lock);
> - list_for_each_entry(e, &smi_infos, link) {
> - if (e->io.dev == dev) {
> - cleanup_one_si(e);
> + list_for_each_entry(tmp, &smi_infos, link) {
> + if (tmp->io.dev == dev) {
> + e = tmp;
> + list_del(&e->link);
> break;
> }
> }
> mutex_unlock(&smi_infos_lock);
> +
> + if (e)
> + cleanup_one_si(e);
> }
>
> struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
> @@ -2377,6 +2413,7 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
> /* remove */
> struct smi_info *e, *tmp_e;
> struct device *dev = NULL;
> + LIST_HEAD(to_clean);
>
> mutex_lock(&smi_infos_lock);
> list_for_each_entry_safe(e, tmp_e, &smi_infos, link) {
> @@ -2386,17 +2423,23 @@ struct device *ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
> continue;
> if (e->io.addr_data == addr) {
> dev = get_device(e->io.dev);
> - cleanup_one_si(e);
> + list_move_tail(&e->link, &to_clean);
> }
> }
> mutex_unlock(&smi_infos_lock);
>
> + list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
> + list_del(&e->link);
> + cleanup_one_si(e);
> + }
> +
> return dev;
> }
>
> static void cleanup_ipmi_si(void)
> {
> struct smi_info *e, *tmp_e;
> + LIST_HEAD(to_clean);
>
> if (!initialized)
> return;
> @@ -2410,10 +2453,14 @@ static void cleanup_ipmi_si(void)
> ipmi_si_platform_shutdown();
>
> mutex_lock(&smi_infos_lock);
> - list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
> - cleanup_one_si(e);
> + list_splice_init(&smi_infos, &to_clean);
> mutex_unlock(&smi_infos_lock);
>
> + list_for_each_entry_safe(e, tmp_e, &to_clean, link) {
> + list_del(&e->link);
> + cleanup_one_si(e);
> + }
> +
> ipmi_si_hardcode_exit();
> ipmi_si_hotmod_exit();
> }
> --
> 2.55.0.654.g21b8a5bc05-goog
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-12 21:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10 7:48 [PATCH v3] ipmi:si: Add async init to ipmi_si Michal Clapinski
2026-08-12 21:29 ` Corey Minyard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox