Re: [PATCH v3] ipmi:si: Add async init to ipmi_si

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 <[email protected]>
> ---
> 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
>