Re: git: 87e140a5c6f8 - main - iwlwifi: avoid (hard) hang on loading module

On 9 Dec 2024, at 14:47, Bjoern A. Zeeb <bz@FreeBSD.org> wrote:
> 
> The branch main has been updated by bz:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=87e140a5c6f89eea7ea6320d1ae34566492abfc0
> 
> commit 87e140a5c6f89eea7ea6320d1ae34566492abfc0
> Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
> AuthorDate: 2024-12-08 20:24:10 +0000
> Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
> CommitDate: 2024-12-09 14:45:24 +0000
> 
>    iwlwifi: avoid (hard) hang on loading module
> 
>    For certain users or chipsets (reports were for CNVi devices but
>    we are not sure if this is limited or specific to them) loading
>    if_iwlwifi hangs.
> 
>    The reason for this is that a SYSINIT (module_load_order()) has not
>    yet run in this case and the Linux driver tries to load the
>    chipsets-specific module.  On FreeBSD all supported sub-modules are
>    part of if_iwlwifi so we do not have to load them separately but
>    calling into kern_kldload via LinuxKPI request_module while loading
>    the module gives us a hard hang.
> 
>    iwlwifi calls request_module_nowait() so we can simply skip over this
>    and continue and the SYSINIT will do the job later if no other
>    dependencies fail.
> 
>    Sponsored by:   The FreeBSD Foundation
>    MFC after:      3 days
>    PR:             282789
>    Tested by:      Ruslan Makhmatkhanov, Pete Wright
>    Differential Revision: https://reviews.freebsd.org/D47994
> ---
> sys/contrib/dev/iwlwifi/iwl-drv.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
> 
> diff --git a/sys/contrib/dev/iwlwifi/iwl-drv.c b/sys/contrib/dev/iwlwifi/iwl-drv.c
> index 7f4746e5591e..61e5c064de80 100644
> --- a/sys/contrib/dev/iwlwifi/iwl-drv.c
> +++ b/sys/contrib/dev/iwlwifi/iwl-drv.c
> @@ -1749,7 +1749,20 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
> goto out_unbind;
> }
> } else {
> +#if defined(__linux__)
> request_module_nowait("%s", op->name);
> +#elif defined(__FreeBSD__)
> + /*
> + * In FreeBSD if_iwlwifi contains all drv modules in a single
> + * module.  SYSINIT will do the job later.  We cannot call
> + * into kern_kldload() while being called from kern_kldload():
> + * LinuxKPI request_module[_nowait] will hang hard.
> + * Given this is request_module_nowait() we can simply skip it.
> + */
> + if (bootverbose)
> +       printf("%s: module '%s' not yet available; will be"
> +   "initialized in a moment\n", __func__, op->name);

Missing a space in the string.

Also, why not just have request_module_nowait itself defer the load
until it can be done, at which point it can notice that there’s nothing
to do?

Jess

> +#endif
> }
> mutex_unlock(&iwlwifi_opmode_table_mtx);
>