LinuxKPI: Rework LINUXKPI_PARAM_charp()

[zlei]

Summary

This was introduced by change [1] but never worked. SYSCTL_STRING
requires a compile-time constant pointer to a string, but
LINUXKPI_PARAM_charp pass in a pointer type char ** .

[1] c1661d59e68e LinuxKPI: add LINUXKPI_PARAM_charp()

Fixes: c1661d59e68e LinuxKPI: add LINUXKPI_PARAM_charp()
MFC after: 1 week

Test Plan

Test preloaded module,

# echo 'compat.linuxkpi.iwlwifi_nvm_file="/tmp/foo"' >> /boot/loader.conf
# echo 'if_iwlwifi_load="YES"' >> /boot/loader.conf
# shutdown -r now
...
# sysctl compat.linuxkpi.iwlwifi_nvm_file
compat.linuxkpi.iwlwifi_nvm_file: /tmp/foo

Test dynamically loaded module,

# kldunload if_iwlwifi
# kenv -u compat.linuxkpi.iwlwifi_nvm_file
# kldload if_iwlwifi
# sysctl compat.linuxkpi.iwlwifi_nvm_file
compat.linuxkpi.iwlwifi_nvm_file:

# kldunload if_iwlwifi
# kenv compat.linuxkpi.iwlwifi_nvm_file=/tmp/bar
# kldload if_iwlwifi
# sysctl compat.linuxkpi.iwlwifi_nvm_file
compat.linuxkpi.iwlwifi_nvm_file: /tmp/bar

[zlei]

Currently the only one consumer of LINUXKPI_PARAM_charp() is iwlwifi. I do not have the hardware so no am unable to test whether the function to load iwlwifi_nvm_file works or not. This is focusing only loader tunable feature.

[kib]

sys/compat/linuxkpi/common/include/linux/moduleparam.h

129

Is KMEM for having malloc working?

sys/compat/linuxkpi/common/src/linux_compat.c

2777

This probably should be sx_sunlock(). What is the point of sysctlcharplock? What does it protect?

2850

Why reallocating and not reusing the initial alloc? Is it to save memory? I suspect it is not too much economy.

[zlei]

sys/compat/linuxkpi/common/include/linux/moduleparam.h

129

Is KMEM for having malloc working?

function linux_tunable_charp_init requires malloc(..., M_KMALLOC), and M_KMALLOC is defined in sys/compat/linuxkpi/common/src/linux_compat.c:

MALLOC_DEFINE(M_KMALLOC, "lkpikmalloc", "Linux kmalloc compat");

while MALLOC_DEFINE in sys/sys/malloc.h:

#ifdef _KERNEL
#define MALLOC_DEFINE(type, shortdesc, longdesc)                        \
        struct malloc_type type[1] = {                                  \
                {                                                       \
                        .ks_next = NULL,                                \
                        .ks_version = M_VERSION,                        \
                        .ks_shortdesc = shortdesc,                      \
                }                                                       \
        };                                                              \
        SYSINIT(type##_init, SI_SUB_KMEM, SI_ORDER_THIRD, malloc_init,  \
            type);                                                      \
        SYSUNINIT(type##_uninit, SI_SUB_KMEM, SI_ORDER_ANY,             \
            malloc_uninit, type)

So YES, SI_SUB_KMEM is required.

146

Is this arbitrary size large enough ?

sys/compat/linuxkpi/common/src/linux_compat.c

2777

This probably should be sx_sunlock(). What is the point of sysctlcharplock? What does it protect?

Ahh, yes, you're right. A copy-paste bug.

2850

The initial alloc may be arbitrary large, I guess it is better to have a max length parameter in LINUXKPI_PARAM_charp()

---#define	LINUXKPI_PARAM_charp(name, var, perm)
+++#define	LINUXKPI_PARAM_charp(name, var, size, perm)

LINUXKPI_PARAM_charp() actually emulate the linux version, I'm not sure if a new parameter size is proper.

[zlei]

sys/compat/linuxkpi/common/src/linux_compat.c

2775

Emm, missing the null terminator, but sysctl(8) tolerates this.

[zlei]

Fixed off-by-one error. Fixed sx_sunlock().

[wulf]

There is prior work to fix that D34252

[zlei]

In D48941#1116247, @wulf wrote:

There is prior work to fix that D34252

Emm, I did not realize that a prior work exists. Should I continue working on this one, or wait for D34252 ?

[wulf]

In D48941#1116640, @zlei wrote:

Emm, I did not realize that a prior work exists. Should I continue working on this one, or wait for D34252 ?

I do not know. It looks abandoned