Re: kern/56568: ipmi.c 1.7 causes large boot delays

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/56568; it has been noted by GNATS.

From: Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>
To: Michael van Elst <mlelstv%NetBSD.org@localhost>,
	Hauke Fath <hf%spg.tu-darmstadt.de@localhost>,
	Edgar =?iso-8859-1?B?RnXf?= <ef%math.uni-bonn.de@localhost>,
	Manuel Bouyer <bouyer%antioche.eu.org@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Subject: Re: kern/56568: ipmi.c 1.7 causes large boot delays
Date: Tue, 3 Dec 2024 01:00:57 +0000

 This is a multi-part message in MIME format.
 --=_wXwkmwLMJljksrXeYBkEj62oCldrq9cV
 
 The attached patch -- compile-tested only -- tries to resolve the
 present conflict between long ipmi(4) initialization time and early
 use of wdogctl(8) at boot by:
 
 (a) allowing boot to proceed with config_pending_decr sooner, before
     device initialization has completed; and
 
 (b) making watchdog configuration wait until initialization has
     completed.
 
 Testing welcome!
 
 --=_wXwkmwLMJljksrXeYBkEj62oCldrq9cV
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr56568-ipmidelay"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr56568-ipmidelay.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1733184792 0
 #      Tue Dec 03 00:13:12 2024 +0000
 # Branch trunk
 # Node ID 32dee81c8e034a77811fddb38816c166f46bb530
 # Parent  9e743b738fb8d4ce4e33ddccdc9acb6e8e81f9df
 # EXP-Topic riastradh-pr56568-ipmidelay
 ipmi(4): Don't hold up boot -- just hold up wdogctl(8).
 
 While here, omit needless volatile qualifier on variables serialized
 by mutex (or by config_pending_incr/decr -- ipmi_detach can't run
 until ipmi_thread does config_pending_decr).
 
 PR kern/56568: ipmi.c 1.7 causes large boot delays
 
 diff -r 9e743b738fb8 -r 32dee81c8e03 sys/dev/ipmi.c
 --- a/sys/dev/ipmi.c	Sun Dec 01 11:10:34 2024 +0000
 +++ b/sys/dev/ipmi.c	Tue Dec 03 00:13:12 2024 +0000
 @@ -1998,6 +1998,23 @@ ipmi_thread(void *cookie)
  	/* Map registers */
  	ipmi_map_regs(sc, ia);
 =20
 +	/* Setup Watchdog timer */
 +	sc->sc_wdog.smw_name =3D device_xname(sc->sc_dev);
 +	sc->sc_wdog.smw_cookie =3D sc;
 +	sc->sc_wdog.smw_setmode =3D ipmi_watchdog_setmode;
 +	sc->sc_wdog.smw_tickle =3D ipmi_watchdog_tickle;
 +	sysmon_wdog_register(&sc->sc_wdog);
 +
 +	/* Set up a power handler so we can possibly sleep */
 +	if (!pmf_device_register(self, ipmi_suspend, NULL))
 +                aprint_error_dev(self, "couldn't establish a power handler=
 \n");
 +
 +	/*
 +	 * Allow boot to proceed -- we'll do the rest asynchronously
 +	 * since it requires talking to the device.
 +	 */
 +	config_pending_decr(self);
 +
  	memset(&id, 0, sizeof(id));
  	if (ipmi_get_device_id(sc, &id))
  		aprint_error_dev(self, "Failed to re-query device ID\n");
 @@ -2098,20 +2115,9 @@ ipmi_thread(void *cookie)
  	/* setup flag to exclude iic */
  	ipmi_enabled =3D 1;
 =20
 -	/* Setup Watchdog timer */
 -	sc->sc_wdog.smw_name =3D device_xname(sc->sc_dev);
 -	sc->sc_wdog.smw_cookie =3D sc;
 -	sc->sc_wdog.smw_setmode =3D ipmi_watchdog_setmode;
 -	sc->sc_wdog.smw_tickle =3D ipmi_watchdog_tickle;
 -	sysmon_wdog_register(&sc->sc_wdog);
 -
 -	/* Set up a power handler so we can possibly sleep */
 -	if (!pmf_device_register(self, ipmi_suspend, NULL))
 -                aprint_error_dev(self, "couldn't establish a power handler=
 \n");
 -
 -	config_pending_decr(self);
 -
  	mutex_enter(&sc->sc_poll_mtx);
 +	sc->sc_thread_ready =3D true;
 +	cv_broadcast(&sc->sc_mode_cv);
  	while (sc->sc_thread_running) {
  		while (sc->sc_mode =3D=3D IPMI_MODE_COMMAND)
  			cv_wait(&sc->sc_mode_cv, &sc->sc_poll_mtx);
 @@ -2259,6 +2265,15 @@ ipmi_watchdog_setmode(struct sysmon_wdog
  	else
  		sc->sc_wdog.smw_period =3D smwdog->smw_period;
 =20
 +	/* Wait until the device is initialized */
 +	rc =3D 0;
 +	mutex_enter(&sc->sc_poll_mtx);
 +	while (sc->sc_thread_ready)
 +		rc =3D cv_wait_sig(&sc->sc_mode_cv, &sc->sc_poll_mtx);
 +	mutex_exit(&sc->sc_poll_mtx);
 +	if (rc)
 +		return rc;
 +
  	mutex_enter(&sc->sc_cmd_mtx);
  	/* see if we can properly task to the watchdog */
  	rc =3D ipmi_sendcmd(sc, BMC_SA, BMC_LUN, APP_NETFN,
 diff -r 9e743b738fb8 -r 32dee81c8e03 sys/dev/ipmivar.h
 --- a/sys/dev/ipmivar.h	Sun Dec 01 11:10:34 2024 +0000
 +++ b/sys/dev/ipmivar.h	Tue Dec 03 00:13:12 2024 +0000
 @@ -101,8 +101,9 @@ struct ipmi_softc {
  	struct ipmi_bmc_args	*sc_iowait_args;
 =20
  	struct ipmi_sensor	*current_sensor;
 -	volatile bool		sc_thread_running;
 -	volatile bool		sc_tickle_due;
 +	bool			sc_thread_running;
 +	bool			sc_thread_ready;
 +	bool			sc_tickle_due;
  	struct sysmon_wdog	sc_wdog;
  	struct sysmon_envsys	*sc_envsys;
  	envsys_data_t		*sc_sensor;
 
 --=_wXwkmwLMJljksrXeYBkEj62oCldrq9cV--