[PATCH v2] ipmi:si: Add async init to ipmi_si

[PATCH v2] ipmi:si: Add async init to ipmi_si

[Michal Clapinski]

Added a new config option to allow offloading individual calls to
try_smi_init(). Saves 100ms on my system.

Signed-off-by: Michal Clapinski <mclapinski@google.com>
---
v2:
- instead of offloading the whole init function, offload just the
  individual calls to try_smi_init()

I didn't implement the periodic retry feature that was talked about
under v1 due to my lack of expertise. LMK if this is a deal-breaker.
---
 drivers/char/ipmi/Kconfig        |  9 +++++++++
 drivers/char/ipmi/ipmi_si_intf.c | 32 ++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index 669f76000197..c8fa445c1c17 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 invidual 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..504d5b8636ba 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/async.h>
 #include "ipmi_si.h"
 #include "ipmi_si_sm.h"
 #include <linux/string.h>
@@ -2174,6 +2175,17 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
 		e1->io.addr_data == e2->io.addr_data);
 }
 
+static ASYNC_DOMAIN_EXCLUSIVE(ipmi_si_async_domain);
+
+static void __init async_try_smi_init(void *data, async_cookie_t cookie)
+{
+	struct smi_info *smi = data;
+
+	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 +2231,13 @@ static int __init init_ipmi_si(void)
 				break;
 			}
 		}
-		if (!dup)
-			try_smi_init(e);
+		if (!dup) {
+			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+				async_schedule_domain(async_try_smi_init, e,
+						      &ipmi_si_async_domain);
+			else
+				try_smi_init(e);
+		}
 	}
 
 	/*
@@ -2253,8 +2270,13 @@ static int __init init_ipmi_si(void)
 				break;
 			}
 		}
-		if (!dup)
-			try_smi_init(e);
+		if (!dup) {
+			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
+				async_schedule_domain(async_try_smi_init, e,
+						      &ipmi_si_async_domain);
+			else
+				try_smi_init(e);
+		}
 	}
 
 	initialized = true;
@@ -2401,6 +2423,8 @@ static void cleanup_ipmi_si(void)
 	if (!initialized)
 		return;
 
+	async_synchronize_full_domain(&ipmi_si_async_domain);
+
 	ipmi_si_pci_shutdown();
 
 	ipmi_si_ls2k_shutdown();
-- 
2.55.0.795.g602f6c329a-goog

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

[Corey Minyard]

On Fri, Jul 03, 2026 at 05:09:55PM +0200, Michal Clapinski wrote:
> Added a new config option to allow offloading individual calls to
> try_smi_init(). Saves 100ms on my system.

This looks good.  In a few releases I may change the default to y, if I
can remember it by then :-).

It's in my next tree.

-corey

> 
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
> v2:
> - instead of offloading the whole init function, offload just the
>   individual calls to try_smi_init()
> 
> I didn't implement the periodic retry feature that was talked about
> under v1 due to my lack of expertise. LMK if this is a deal-breaker.
> ---
>  drivers/char/ipmi/Kconfig        |  9 +++++++++
>  drivers/char/ipmi/ipmi_si_intf.c | 32 ++++++++++++++++++++++++++++----
>  2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 669f76000197..c8fa445c1c17 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 invidual 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..504d5b8636ba 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/async.h>
>  #include "ipmi_si.h"
>  #include "ipmi_si_sm.h"
>  #include <linux/string.h>
> @@ -2174,6 +2175,17 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
>  		e1->io.addr_data == e2->io.addr_data);
>  }
>  
> +static ASYNC_DOMAIN_EXCLUSIVE(ipmi_si_async_domain);
> +
> +static void __init async_try_smi_init(void *data, async_cookie_t cookie)
> +{
> +	struct smi_info *smi = data;
> +
> +	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 +2231,13 @@ static int __init init_ipmi_si(void)
>  				break;
>  			}
>  		}
> -		if (!dup)
> -			try_smi_init(e);
> +		if (!dup) {
> +			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> +				async_schedule_domain(async_try_smi_init, e,
> +						      &ipmi_si_async_domain);
> +			else
> +				try_smi_init(e);
> +		}
>  	}
>  
>  	/*
> @@ -2253,8 +2270,13 @@ static int __init init_ipmi_si(void)
>  				break;
>  			}
>  		}
> -		if (!dup)
> -			try_smi_init(e);
> +		if (!dup) {
> +			if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> +				async_schedule_domain(async_try_smi_init, e,
> +						      &ipmi_si_async_domain);
> +			else
> +				try_smi_init(e);
> +		}
>  	}
>  
>  	initialized = true;
> @@ -2401,6 +2423,8 @@ static void cleanup_ipmi_si(void)
>  	if (!initialized)
>  		return;
>  
> +	async_synchronize_full_domain(&ipmi_si_async_domain);
> +
>  	ipmi_si_pci_shutdown();
>  
>  	ipmi_si_ls2k_shutdown();
> -- 
> 2.55.0.795.g602f6c329a-goog
> 

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

[Michał Cłapiński]

On Fri, Jul 3, 2026 at 5:11 PM Michal Clapinski <mclapinski@google.com> wrote:
>
> Added a new config option to allow offloading individual calls to
> try_smi_init(). Saves 100ms on my system.
>
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
> v2:
> - instead of offloading the whole init function, offload just the
>   individual calls to try_smi_init()
>
> I didn't implement the periodic retry feature that was talked about
> under v1 due to my lack of expertise. LMK if this is a deal-breaker.
> ---
>  drivers/char/ipmi/Kconfig        |  9 +++++++++
>  drivers/char/ipmi/ipmi_si_intf.c | 32 ++++++++++++++++++++++++++++----
>  2 files changed, 37 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 669f76000197..c8fa445c1c17 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 invidual 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..504d5b8636ba 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/async.h>
>  #include "ipmi_si.h"
>  #include "ipmi_si_sm.h"
>  #include <linux/string.h>
> @@ -2174,6 +2175,17 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
>                 e1->io.addr_data == e2->io.addr_data);
>  }
>
> +static ASYNC_DOMAIN_EXCLUSIVE(ipmi_si_async_domain);
> +
> +static void __init async_try_smi_init(void *data, async_cookie_t cookie)
> +{
> +       struct smi_info *smi = data;
> +
> +       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 +2231,13 @@ static int __init init_ipmi_si(void)
>                                 break;
>                         }
>                 }
> -               if (!dup)
> -                       try_smi_init(e);
> +               if (!dup) {
> +                       if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> +                               async_schedule_domain(async_try_smi_init, e,
> +                                                     &ipmi_si_async_domain);
> +                       else
> +                               try_smi_init(e);
> +               }
>         }
>
>         /*
> @@ -2253,8 +2270,13 @@ static int __init init_ipmi_si(void)
>                                 break;
>                         }
>                 }
> -               if (!dup)
> -                       try_smi_init(e);
> +               if (!dup) {
> +                       if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> +                               async_schedule_domain(async_try_smi_init, e,
> +                                                     &ipmi_si_async_domain);
> +                       else
> +                               try_smi_init(e);
> +               }
>         }
>
>         initialized = true;
> @@ -2401,6 +2423,8 @@ static void cleanup_ipmi_si(void)
>         if (!initialized)
>                 return;
>
> +       async_synchronize_full_domain(&ipmi_si_async_domain);
> +
>         ipmi_si_pci_shutdown();
>
>         ipmi_si_ls2k_shutdown();

I've reviewed comments by sashiko:
https://sashiko.dev/#/patchset/20260703150955.3943082-1-mclapinski%40google.com.
It makes 3 points:

1. async_try_smi_init is marked __init but it shouldn't be.
That's valid. I'll fix that in v3.

2. async_schedule_domain can run code synchronously. That would result
in a deadlock.

That is true however async_schedule_domain currently only runs
synchronously if we're out of memory or there are 32k functions
scheduled. It's not very probable but still worth fixing.

3. I'm passing the struct "e" by pointer to an async function. In the
meantime the lock guarding it will be released. Could that pointer
become invalid before we acquire the lock again?

That again doesn't seem very probable.

I'm not sure how to proceed to fix points 2 and 3. v1 where I made the
whole init_ipmi_si async was immune to those problems so maybe we
could go back to it?
Alternatively I could keep the lock acquired and only release it when
the last async_try_smi_init function finishes but that's obviously
more complicated than just returning to v1.

What do you think?

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

[Corey Minyard]

On Wed, Aug 05, 2026 at 04:55:42PM +0200, Michał Cłapiński wrote:
> On Fri, Jul 3, 2026 at 5:11 PM Michal Clapinski <mclapinski@google.com> wrote:
> >
> > Added a new config option to allow offloading individual calls to
> > try_smi_init(). Saves 100ms on my system.
> >
> > Signed-off-by: Michal Clapinski <mclapinski@google.com>
> > ---
> > v2:
> > - instead of offloading the whole init function, offload just the
> >   individual calls to try_smi_init()
> >
> > I didn't implement the periodic retry feature that was talked about
> > under v1 due to my lack of expertise. LMK if this is a deal-breaker.
> > ---
> >  drivers/char/ipmi/Kconfig        |  9 +++++++++
> >  drivers/char/ipmi/ipmi_si_intf.c | 32 ++++++++++++++++++++++++++++----
> >  2 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> > index 669f76000197..c8fa445c1c17 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 invidual 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..504d5b8636ba 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/async.h>
> >  #include "ipmi_si.h"
> >  #include "ipmi_si_sm.h"
> >  #include <linux/string.h>
> > @@ -2174,6 +2175,17 @@ static bool __init ipmi_smi_info_same(struct smi_info *e1, struct smi_info *e2)
> >                 e1->io.addr_data == e2->io.addr_data);
> >  }
> >
> > +static ASYNC_DOMAIN_EXCLUSIVE(ipmi_si_async_domain);
> > +
> > +static void __init async_try_smi_init(void *data, async_cookie_t cookie)
> > +{
> > +       struct smi_info *smi = data;
> > +
> > +       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 +2231,13 @@ static int __init init_ipmi_si(void)
> >                                 break;
> >                         }
> >                 }
> > -               if (!dup)
> > -                       try_smi_init(e);
> > +               if (!dup) {
> > +                       if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> > +                               async_schedule_domain(async_try_smi_init, e,
> > +                                                     &ipmi_si_async_domain);
> > +                       else
> > +                               try_smi_init(e);
> > +               }
> >         }
> >
> >         /*
> > @@ -2253,8 +2270,13 @@ static int __init init_ipmi_si(void)
> >                                 break;
> >                         }
> >                 }
> > -               if (!dup)
> > -                       try_smi_init(e);
> > +               if (!dup) {
> > +                       if (IS_ENABLED(CONFIG_IPMI_SI_ASYNC_INIT))
> > +                               async_schedule_domain(async_try_smi_init, e,
> > +                                                     &ipmi_si_async_domain);
> > +                       else
> > +                               try_smi_init(e);
> > +               }
> >         }
> >
> >         initialized = true;
> > @@ -2401,6 +2423,8 @@ static void cleanup_ipmi_si(void)
> >         if (!initialized)
> >                 return;
> >
> > +       async_synchronize_full_domain(&ipmi_si_async_domain);
> > +
> >         ipmi_si_pci_shutdown();
> >
> >         ipmi_si_ls2k_shutdown();
> 
> I've reviewed comments by sashiko:
> https://sashiko.dev/#/patchset/20260703150955.3943082-1-mclapinski%40google.com.

Hmm, I didn't get this.  That's strange.  I see that I was cc-ed, but I
don't have that email.

> It makes 3 points:
> 
> 1. async_try_smi_init is marked __init but it shouldn't be.
> That's valid. I'll fix that in v3.

Yeah, I missed that.

> 
> 2. async_schedule_domain can run code synchronously. That would result
> in a deadlock.
> 
> That is true however async_schedule_domain currently only runs
> synchronously if we're out of memory or there are 32k functions
> scheduled. It's not very probable but still worth fixing.

I didn't realize this.  That could be a problem.

You could use a nested mutex lock.  Not my favorite thing, but it's an
easy solution.

Other mechanisms exist to run things asynchronously, like workqueues.  I
don't think queue_work_on() will ever run synchronously.  It might also
help the next problem.

> 
> 3. I'm passing the struct "e" by pointer to an async function. In the
> meantime the lock guarding it will be released. Could that pointer
> become invalid before we acquire the lock again?
> 
> That again doesn't seem very probable.

As Sashiko pointed out, it could theoretically be hot-removed before
this runs.  Not likely, but I'm working on a modification on top of this
to retry if this fails.  Then it will become a lot more likely.

The right way to fix this, of course, is to add a refcount and a flag in
the structure to say it's deleted.

Or, if you had a workqueue, you could cancel and flush the work, I
think, to make sure it wasn't running, before doing the remove
operation.

> 
> I'm not sure how to proceed to fix points 2 and 3. v1 where I made the
> whole init_ipmi_si async was immune to those problems so maybe we
> could go back to it?

We could do that, but it's just going to end up being rewritten

> Alternatively I could keep the lock acquired and only release it when
> the last async_try_smi_init function finishes but that's obviously
> more complicated than just returning to v1.

Yeah, that's not a great solution.  You really don't want to be locking
and unlocking mutexes in different threads.

> 
> What do you think?

If you want to take a try at it, you can.  I can try to handle all this
if you like.

-corey