* Re: [RFC PATCH 00/10] scalable symbol flags with __kflagstab
From: Petr Pavlu @ 2025-09-22 11:41 UTC (permalink / raw)
To: Sid Nayyar
Cc: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen,
Nicolas Schier, Arnd Bergmann, linux-kbuild, linux-arch,
linux-modules, linux-kernel, Giuliano Procida,
Matthias Männich
In-Reply-To: <CA+OvW8bhWK7prmyQMMJ_VYBeGMbn_mNiamHhUgYuCsnht+LFtA@mail.gmail.com>
On 9/15/25 5:53 PM, Sid Nayyar wrote:
> On Mon, Sep 8, 2025 at 11:09 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>> This sounds reasonable to me. Do you have any numbers on hand that would
>> show the impact of extending __ksymtab?
>
> I did performance analysis for module loading. The kflagstab
> optimizes symbol search, which accounts for less than 2% of the
> average module load time. Therefore, this change does not translate
> into any meaningful gains (or losses) in module loading performance.
>
> On the binary size side, the on-disk size for vmlinux is somewhat
> inflated due to extra entries in .symtab and .strtab. Since these
> sections are not part of the final Image, the only increase in the
> in-memory size of the kernel is for the kflagstab itself. This new
> table occupies 1 byte for each symbol in the ksymtab.
This is useful information. However, I was specifically interested in
the impact of having the new flags field present as part of __ksymtab
(kernel_symbol), compared to keeping it in a separate section. Sorry for
not being clear.
I ran a small test to get a better understanding of the different sizes.
I used v6.17-rc6 together with the openSUSE x86_64 config [1], which is
fairly large. The resulting vmlinux.bin (no debuginfo) had an on-disk
size of 58 MiB, and included 5937 + 6589 (GPL-only) exported symbols.
The following table summarizes my measurements and calculations
regarding the sizes of all sections related to exported symbols:
| HAVE_ARCH_PREL32_RELOCATIONS | !HAVE_ARCH_PREL32_RELOCATIONS
Section | Base [B] | Ext. [B] | Sep. [B] | Base [B] | Ext. [B] | Sep. [B]
----------------------------------------------------------------------------------------
__ksymtab | 71244 | 200416 | 150312 | 142488 | 400832 | 300624
__ksymtab_gpl | 79068 | NA | NA | 158136 | NA | NA
__kcrctab | 23748 | 50104 | 50104 | 23748 | 50104 | 50104
__kcrctab_gpl | 26356 | NA | NA | 26356 | NA | NA
__ksymtab_strings | 253628 | 253628 | 253628 | 253628 | 253628 | 253628
__kflagstab | NA | NA | 12526 | NA | NA | 12526
----------------------------------------------------------------------------------------
Total | 454044 | 504148 | 466570 | 604356 | 704564 | 616882
Increase to base [%] | NA | 11.0 | 2.8 | NA | 16.6 | 2.1
The column "HAVE_ARCH_PREL32_RELOCATIONS -> Base" contains the numbers
that I measured. The rest of the values are calculated. The "Ext."
column represents the variant of extending __ksymtab, and the "Sep."
column represents the variant of having a separate __kflagstab. With
HAVE_ARCH_PREL32_RELOCATIONS, each kernel_symbol is 12 B in size and is
extended to 16 B. With !HAVE_ARCH_PREL32_RELOCATIONS, it is 24 B,
extended to 32 B. Note that this does not include the metadata needed to
relocate __ksymtab*, which is freed after the initial processing.
The base export data in this case totals 0.43 MiB. About 50% is used for
storing the names of exported symbols.
Adding __kflagstab as a separate section has a negligible impact, as
expected. When extending __ksymtab (kernel_symbol) instead, the worst
case with !HAVE_ARCH_PREL32_RELOCATIONS increases the export data size
by 16.6%.
Based on the above, I think introducing __kflagstab makes senses, as the
added complexity is minimal, although I feel we could probably also get
away with extending kernel_symbol.
>
>>> The Android Common Kernel source is compiled into what we call
>>> GKI (Generic Kernel Image), which consists of a kernel and a
>>> number of modules. We maintain a stable interface (based on CRCs and
>>> types) between the GKI components and vendor-specific modules
>>> (compiled by device manufacturers, e.g., for hardware-specific
>>> drivers) for the lifetime of a given GKI version.
>>>
>>> This interface is intentionally restricted to the minimal set of
>>> symbols required by the union of all vendor modules; our partners
>>> declare their requirements in symbol lists. Any additions to these
>>> lists are reviewed to ensure kernel internals are not overly exposed.
>>> For example, we restrict drivers from having the ability to open and
>>> read arbitrary files. This ABI boundary also allows us to evolve
>>> internal kernel types that are not exposed to vendor modules, for
>>> example, when a security fix requires a type to change.
>>>
>>> The mechanism we use for this is CONFIG_TRIM_UNUSED_KSYMS and
>>> CONFIG_UNUSED_KSYMS_WHITELIST. This results in a ksymtab
>>> containing two kinds of exported symbols: those explicitly required
>>> by vendors ("vendor-listed") and those only required by GKI modules
>>> ("GKI use only").
>>>
>>> On top of this, we have implemented symbol import protection
>>> (covered in patches 9/10 and 10/10). This feature prevents vendor
>>> modules from using symbols that are not on the vendor-listed
>>> whitelist. It is built on top of CONFIG_MODULE_SIG. GKI modules are
>>> signed with a specific key, while vendor modules are unsigned and thus
>>> treated as untrusted. This distinction allows signed GKI modules to
>>> use any symbol in the ksymtab, while unsigned vendor modules can only
>>> access the declared subset. This provides a significant layer of
>>> defense and security against potentially exploitable vendor module
>>> code.
>>
>> If I understand correctly, this is similar to the recently introduced
>> EXPORT_SYMBOL_FOR_MODULES() macro, but with a coarser boundary.
>>
>> I think that if the goal is to control the kABI scope and limit the use
>> of certain symbols only to GKI modules, then having the protection
>> depend on whether the module is signed is somewhat odd. It doesn't give
>> me much confidence if vendor modules are unsigned in the Android
>> ecosystem. I would expect that you want to improve this in the long
>> term.
>
> GKI modules are the only modules built in the same Kbuild as the
> kernel image, which Google builds and provides to partners. In
> contrast, vendor modules are built and packaged entirely by partners.
>
> Google signs GKI modules with ephemeral keys. Since partners do
> not have these keys, vendor modules are treated as unsigned by
> the kernel.
>
> To ensure the authenticity of these unsigned modules, partners
> package them into a separate image that becomes one of the boot
> partitions. This entire image is signed, and its signature is
> verified by the bootloader at boot time.
>
>> It would then make more sense to me if the protection was determined by
>> whether the module is in-tree (the "intree" flag in modinfo) or,
>> alternatively, if it is signed by a built-in trusted key. I feel this
>> way the feature could be potentially useful for other distributions that
>> care about the kABI scope and have ecosystems where vendor modules are
>> properly signed with some key. However, I'm not sure if this would still
>> work in your case.
>
> Partners can produce both in-tree and out-of-tree modules. We do not
> trust either type regarding symbol exposure, as there is no way to know
> exactly what sources were used. Furthermore, symbols exported via
> EXPORT_SYMBOL_FOR_MODULES can be accessed by any vendor module that
> mimics the GKI module name.
>
> Therefore, neither the in-tree flag nor the EXPORT_SYMBOL_FOR_MODULES
> mechanism provides a strong enough guarantee for the Android kernel to
> identify GKI modules.
>
> Only module signatures are sufficient to allow a module to access the
> full set of exported symbols. Unsigned vendor modules may only access
> the symbol subset declared ahead of time by partners.
This seems to answer why the in-tree flag is not sufficient for you.
However, I also suggested an alternative that the symbol protection
could be determined by whether the module is signed by a key from the
.builtin_trusted_keys keyring, as opposed to being signed by another key
reachable from the .secondary_trusted_keys keyring or being completely
unsigned.
Distributions can require that external modules be signed and allow
additional keys to be added as Machine Owner Keys, which can be made
reachable from .secondary_trusted_keys. Nonetheless, such distributions
might be still interested in limiting the number of symbols that such
external modules can use.
I think this option is worth considering, as it could potentially make
this symbol protection useful for other distributions as well.
>
> In case such symbol protection is not useful for the Linux community, I
> am happy to keep this as an Android-specific feature. However, I would
> urge you to at least accept the kflagstab, as it allows us (and
> potentially other Linux distributions) to easily introduce additional
> flags for symbols. It is also a simplification/clean-up of the module
> loader code.
I'm personally ok with adding the kflagstab support. I think it
introduces minimal complexity and, as you point out, simplifies certain
aspects. Additionally, if we add it, I believe that adding the proposed
symbol protection is simple enough to be included as well, at least from
my perspective.
[1] https://github.com/openSUSE/kernel-source/blob/307f149d9100a0e229eb94cbb997ae61187995c3/config/x86_64/default
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags
From: Takashi Iwai @ 2025-09-19 12:47 UTC (permalink / raw)
To: cryolitia
Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules, Takashi Iwai
In-Reply-To: <20250918-sound-v4-3-82cf8123d61c@uniontech.com>
On Thu, 18 Sep 2025 11:24:32 +0200,
Cryolitia PukNgae via B4 Relay wrote:
>
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
>
> For apply and unapply quirk flags more flexibly though param
>
> Co-developed-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> ---
> sound/usb/card.c | 9 ++--
> sound/usb/quirks.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> sound/usb/quirks.h | 3 +-
> sound/usb/usbaudio.h | 3 ++
> 4 files changed, 126 insertions(+), 8 deletions(-)
>
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 0265206a8e8cf31133e8463c98fe0497d8ace89e..5837677effa1787ef9d7d02714c3ae43b1a8bc79 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -73,8 +73,8 @@ static bool lowlatency = true;
> static char *quirk_alias[SNDRV_CARDS];
> static char *delayed_register[SNDRV_CARDS];
> static bool implicit_fb[SNDRV_CARDS];
> -static unsigned int quirk_flags[SNDRV_CARDS];
>
> +char *quirk_flags[SNDRV_CARDS];
My preference is to keep this static, but pass the string value to a
function. That is, define snd_usb_init_quirk_flags() in main.c:
static void snd_usb_init_quirk_flags(struct snd_usb_audio *chip, int indx)
{
/* old style option found: the position-based integer value */
if (quirk_flags[idx] &&
!kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
usb_audio_dbg(chip,
"Set quirk flags 0x%x from param based on position %d for device %04x:%04x\n",
chip->quirk_flags, idx,
USB_ID_VENDOR(chip->usb_id),
USB_ID_PRODUCT(chip->usb_id));
return;
}
/* take the default quirk from the quirk table */
snd_usb_init_quirk_flags_table(chip);
/* add or correct quirk bits from options */
for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
char *val __free(kfree) = NULL;
if (!quirk_flags[i] || !*quirk_flags[i])
continue;
val = kstrdup(quirk_flags[i], GFP_KERNEL);
if (!val)
return;
snd_usb_parse_quirk_string(chip, val);
}
}
static int snd_usb_audio_create(....)
{
....
snd_usb_init_quirk_flags(chip, idx);
....
}
The function snd_usb_parse_quirk_string() is defined in quirks.c,
void snd_usb_parse_quirk_string(struct snd_usb_audio *chip, char *val)
{
for (p = val; p && *p;) {
/* Each entry consists of VID:PID:flags */
field = strsep(&p, ":");
if (!field)
break;
.....
}
}
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH v4 2/5] param: export param_array related functions
From: Takashi Iwai @ 2025-09-19 12:37 UTC (permalink / raw)
To: cryolitia
Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules
In-Reply-To: <20250918-sound-v4-2-82cf8123d61c@uniontech.com>
On Thu, 18 Sep 2025 11:24:31 +0200,
Cryolitia PukNgae via B4 Relay wrote:
>
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
>
> - int param_array_set(const char *val, const struct kernel_param *kp);
> - int param_array_get(char *buffer, const struct kernel_param *kp);
> - void param_array_free(void *arg);
>
> It would be helpful for the new module param we designed in
> snd_usb_audio, in order to run additional custom codes when params
> are set in runtime, and re-use the extisted codes in param.c
>
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
Can we do just like below?
static int param_set_quirkp(const char *val, const struct kernel_param *kp)
{
guard(mutex)(&quirk_flags_mutex);
return param_set_charp(val, kp);
}
static const struct kernel_param_ops param_ops_quirkp = {
.set = param_set_quirkp,
.get = param_get_charp,
.free = param_free_charp,
};
#define param_check_quirkp param_check_charp
modules_param_parray(quirk_flags, quirkp, NULL, 0644);
Then mutex is locked at each time a parameter is set.
Optionally, the string value can be verified in param_set_quirkp()
before passing to param_set_charp(), too.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
From: Takashi Iwai @ 2025-09-19 12:32 UTC (permalink / raw)
To: cryolitia
Cc: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, linux-sound, linux-usb,
linux-kernel, linux-doc, Mingcong Bai, Kexy Biscuit, Nie Cheng,
Zhan Jun, Feng Yuan, qaqland, kernel, linux-modules
In-Reply-To: <20250918-sound-v4-1-82cf8123d61c@uniontech.com>
On Thu, 18 Sep 2025 11:24:30 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -2446,6 +2446,62 @@ static const struct usb_audio_quirk_flags_table quirk_flags_table[] = {
> {} /* terminator */
> };
>
> +static const char *const snd_usb_audio_quirk_flag_names[] = {
> + "get_sample_rate",
> + "share_media_device",
I think it's worth to add a comment that this each entry corresponds
to QUIRK_FLAG_XXX. Or another idea would be to define enums like:
enum {
QUIRK_TYPE_GET_SAMPLE_RATE,
QUIRK_TYPE_SHARE_MEDIA_DEVICE,
....
};
then redefine QUIRK_FLAG_* like:
#define QUIRK_FLAG_GET_SAMPLE_RATE BIT_U32(QUIRK_TYPE_GET_SAMPLE_RATE)
#define QUIRK_FLAG_SHARE_MEDIA_DEVICE BIT_U32(QUIRK_TYPE_SHARE_MEDIA_DEVICE)
....
or
#define QUIRK_FLAG(x) BIT_U32(QUIRK_TYPE_ ## x)
and use like QUIRK_FLAG(GET_SAMPLE_RATE).
With those changes, the above can be defined more safely like
static const char *const snd_usb_audio_quirk_flag_names[] = {
[QUIRK_TYPE_GET_SAMPLE_RATE] = "get_sample_rate",
....
or even more drastically by defining some macro for each entry like:
#define QUIRK_STRING_ENTRY(x) \
[QUIRK_TYPE_ ## x] = __stringify(x)
and put like:
static const char *const snd_usb_audio_quirk_flag_names[] = {
QUIRK_STRING_ENTRY(GET_SAMPLE_RATE),
....
};
In this case, it'll become upper letters, so the parse would need to
deal with the case-insensitive comparison, though.
> +u32 snd_usb_quirk_flags_from_name(char *name)
Use const char *.
> +{
> + u32 flag = 0;
> + u32 i;
The iterator can be simple int.
> + if (!name || !*name)
> + return 0;
> +
> + for (i = 0; snd_usb_audio_quirk_flag_names[i]; i++) {
> + if (strcmp(name, snd_usb_audio_quirk_flag_names[i]) == 0) {
> + flag = (1U << i);
Use BIT_U32(i)
> + break;
We can return the value directly, so flag variable can be dropped.
> void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
> {
> const struct usb_audio_quirk_flags_table *p;
> @@ -2454,10 +2510,28 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
> if (chip->usb_id == p->id ||
> (!USB_ID_PRODUCT(p->id) &&
> USB_ID_VENDOR(chip->usb_id) == USB_ID_VENDOR(p->id))) {
> - usb_audio_dbg(chip,
> - "Set quirk_flags 0x%x for device %04x:%04x\n",
> - p->flags, USB_ID_VENDOR(chip->usb_id),
> - USB_ID_PRODUCT(chip->usb_id));
> + unsigned long flags = p->flags;
> + unsigned long bit;
> +
> + for_each_set_bit(bit, &flags,
> + BYTES_TO_BITS(sizeof(p->flags))) {
> + const char *name =
> + snd_usb_audio_quirk_flag_names[bit];
> +
> + if (name)
> + usb_audio_dbg(chip,
> + "Set quirk flag %s for device %04x:%04x\n",
> + name,
> + USB_ID_VENDOR(chip->usb_id),
> + USB_ID_PRODUCT(chip->usb_id));
> + else
> + usb_audio_warn(chip,
> + "Set unknown quirk flag 0x%lx for device %04x:%04x\n",
> + bit,
> + USB_ID_VENDOR(chip->usb_id),
> + USB_ID_PRODUCT(chip->usb_id));
> + }
This could be better factored out as a function.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH v2 1/1] module: replace use of system_wq with system_dfl_wq
From: Petr Pavlu @ 2025-09-19 8:18 UTC (permalink / raw)
To: Marco Crivellari
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Michal Hocko, Luis Chamberlain,
linux-kernel, linux-modules
In-Reply-To: <20250918085525.122429-2-marco.crivellari@suse.com>
On 9/18/25 10:55 AM, Marco Crivellari wrote:
> Currently if a user enqueue a work item using schedule_delayed_work() the
> used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
> WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
> schedule_work() that is using system_wq and queue_work(), that makes use
> again of WORK_CPU_UNBOUND.
>
> This lack of consistentcy cannot be addressed without refactoring the API.
>
> This specific patch replace system_wq with system_dfl_wq, the new unbound
> workqueue, because the users does not benefit from a per-cpu wq.
>
> Suggested-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
From: Cryolitia PukNgae @ 2025-09-19 1:43 UTC (permalink / raw)
To: Randy Dunlap, Jaroslav Kysela, Takashi Iwai, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules
In-Reply-To: <dcbd2c62-5db8-4eb5-aa3a-532b33baaa61@infradead.org>
Thanks for your review. I'll waiting some reviews on other patches and resend
a new version about it.
On 19/09/2025 04.21, Randy Dunlap wrote:
> Hi--
>
> On 9/18/25 2:24 AM, Cryolitia PukNgae via B4 Relay wrote:
>> From: Cryolitia PukNgae <cryolitia@uniontech.com>
>>
>> Just briefly described about the option.
>>
>> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
>> ---
>> Documentation/sound/alsa-configuration.rst | 108 ++++++++++++++++++++---------
>> 1 file changed, 75 insertions(+), 33 deletions(-)
>>
>> diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
>> index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
>> --- a/Documentation/sound/alsa-configuration.rst
>> +++ b/Documentation/sound/alsa-configuration.rst
>> @@ -2297,39 +2297,81 @@ skip_validation
>> of the unit descriptor instead of a driver probe error, so that we
>> can check its details.
>> quirk_flags
>> - Contains the bit flags for various device specific workarounds.
>> - Applied to the corresponding card index.
>> -
>> - * bit 0: Skip reading sample rate for devices
>> - * bit 1: Create Media Controller API entries
>> - * bit 2: Allow alignment on audio sub-slot at transfer
>> - * bit 3: Add length specifier to transfers
>> - * bit 4: Start playback stream at first in implement feedback mode
>> - * bit 5: Skip clock selector setup
>> - * bit 6: Ignore errors from clock source search
>> - * bit 7: Indicates ITF-USB DSD based DACs
>> - * bit 8: Add a delay of 20ms at each control message handling
>> - * bit 9: Add a delay of 1-2ms at each control message handling
>> - * bit 10: Add a delay of 5-6ms at each control message handling
>> - * bit 11: Add a delay of 50ms at each interface setup
>> - * bit 12: Perform sample rate validations at probe
>> - * bit 13: Disable runtime PM autosuspend
>> - * bit 14: Ignore errors for mixer access
>> - * bit 15: Support generic DSD raw U32_BE format
>> - * bit 16: Set up the interface at first like UAC1
>> - * bit 17: Apply the generic implicit feedback sync mode
>> - * bit 18: Don't apply implicit feedback sync mode
>> - * bit 19: Don't closed interface during setting sample rate
>> - * bit 20: Force an interface reset whenever stopping & restarting
>> - a stream
>> - * bit 21: Do not set PCM rate (frequency) when only one rate is
>> - available for the given endpoint.
>> - * bit 22: Set the fixed resolution 16 for Mic Capture Volume
>> - * bit 23: Set the fixed resolution 384 for Mic Capture Volume
>> - * bit 24: Set minimum volume control value as mute for devices
>> - where the lowest playback value represents muted state instead
>> - of minimum audible volume
>> - * bit 25: Be similar to bit 24 but for capture streams
>> + The option provides a refined and flexible control for applying quirk
>> + flags. It allows to specify the quirk flags for each device, and could
>
> and may
> or: and can
>
>> + be modified dynamically via sysfs.
>> + The old usage accepts an array of integers, each of which apply quirk
>
> applies
>
>> + flags on the device in the order of probing.
>> + e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first
>
> E.g.,
>
>> + device, and share_media_device to the second device.
>> + The new usage accepts a string in the format of
>> + ``VID1:PID1:FLAGS1;VID2:PID2:FLAGS2;...``, where ``VIDx`` and ``PIDx``
>> + specify the device, and ``FLAGSx`` specify the flags to be applied.
>> + ``VIDx`` and ``PIDx`` are 4-digit hexadecimal numbers, and could be
>
> s/could/may/
>
>> + specified as ``*`` to match any value. ``FLAGSx`` could be a set of
>
> s/could/may/
>
>> + flags given by name, separated by ``|``, or a hexadecimal number
>> + representing the bit flags. The available flag names are listed above.
>
> s/above/below/ ?
>
>> + An exclamation mark could be prefixed to a flag name to negate the flag.
> s/could/may/
>
>> + For example, ``1234:abcd:mixer_playback_min_mute|!ignore_ctl_error;*:*:0x01;``
>
> What happens if the trailing (ending) ';' is omitted?
This is where I found something strange when I was testing it myself. When I use
`echo '*:*:mixer_playback_min_mute` > /sys/module/snd_usb_audio/parameters/quirk_flags`,
the string received by the driver contains a lot of trailing spaces. I am not familiar
with the kernel and don't know how to handle this situation. Simply adding a semicolon
to the input will make the trailing spaces after the semicolon be ignored.
>
>> + applies the ``mixer_playback_min_mute`` flag and clears the
>> + ``ignore_ctl_error`` flag for the device 1234:abcd, and applies the
>> + ``skip_sample_rate`` flag for all devices.
>> +
>> + * bit 0: ``get_sample_rate``
>> + Skip reading sample rate for devices
>
> get vs Skip is a little confusing.
This part is copied from Takashi Iwai[1]. It does a little confusing.
>> + * bit 1: ``share_media_device``
>> + Create Media Controller API entries
>> + * bit 2: ``align_transfer``
>> + Allow alignment on audio sub-slot at transfer
>> + * bit 3: ``tx_length``
>> + Add length specifier to transfers
>> + * bit 4: ``playback_first``
>> + Start playback stream at first in implement feedback mode
>> + * bit 5: ``skip_clock_selector``
>> + Skip clock selector setup
>> + * bit 6: ``ignore_clock_source``
>> + Ignore errors from clock source search
>> + * bit 7: ``itf_usb_dsd_dac``
>> + Indicates ITF-USB DSD based DACs
>
> DSD-based
>
>> + * bit 8: ``ctl_msg_delay``
>> + Add a delay of 20ms at each control message handling
>> + * bit 9: ``ctl_msg_delay_1m``
>> + Add a delay of 1-2ms at each control message handling
>> + * bit 10: ``ctl_msg_delay_5m``
>> + Add a delay of 5-6ms at each control message handling
>> + * bit 11: ``iface_delay``
>> + Add a delay of 50ms at each interface setup
>> + * bit 12: ``validate_rates``
>> + Perform sample rate validations at probe
>> + * bit 13: ``disable_autosuspend``
>> + Disable runtime PM autosuspend
>> + * bit 14: ``ignore_ctl_error``
>> + Ignore errors for mixer access
>> + * bit 15: ``dsd_raw``
>> + Support generic DSD raw U32_BE format
>> + * bit 16: ``set_iface_first``
>> + Set up the interface at first like UAC1
>> + * bit 17: ``generic_implicit_fb``
>> + Apply the generic implicit feedback sync mode
>> + * bit 18: ``skip_implicit_fb``
>> + Don't apply implicit feedback sync mode
>> + * bit 19: ``iface_skip_close``
>> + Don't closed interface during setting sample rate
>
> close
>
>> + * bit 20: ``force_iface_reset``
>> + Force an interface reset whenever stopping & restarting a stream
>> + * bit 21: ``fixed_rate``
>> + Do not set PCM rate (frequency) when only one rate is available
>> + for the given endpoint
>> + * bit 22: ``mic_res_16``
>> + Set the fixed resolution 16 for Mic Capture Volume
>> + * bit 23: ``mic_res_384``
>> + Set the fixed resolution 384 for Mic Capture Volume
>> + * bit 24: ``mixer_playback_min_mute``
>> + Set minimum volume control value as mute for devices where the
>> + lowest playback value represents muted state instead of minimum
>> + audible volume
>> + * bit 25: ``mixer_capture_min_mute``
>> + Be similar to bit 24 but for capture streams
>
> Similar to
>
>>
>> This module supports multiple devices, autoprobe and hotplugging.
>>
>> Are all of these quirks used on various devices or are some of these
> just implemented just in case they are needed in the future?thanks.
I believe in that all of them are used in quirk_flags_table in sound/usb/quirks.c
1. https://lore.kernel.org/all/20210729073855.19043-2-tiwai@suse.de/
Best regards,
Cryolitia
^ permalink raw reply
* Re: [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
From: Randy Dunlap @ 2025-09-18 20:21 UTC (permalink / raw)
To: cryolitia, Jaroslav Kysela, Takashi Iwai, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules
In-Reply-To: <20250918-sound-v4-5-82cf8123d61c@uniontech.com>
Hi--
On 9/18/25 2:24 AM, Cryolitia PukNgae via B4 Relay wrote:
> From: Cryolitia PukNgae <cryolitia@uniontech.com>
>
> Just briefly described about the option.
>
> Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
> ---
> Documentation/sound/alsa-configuration.rst | 108 ++++++++++++++++++++---------
> 1 file changed, 75 insertions(+), 33 deletions(-)
>
> diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
> index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
> --- a/Documentation/sound/alsa-configuration.rst
> +++ b/Documentation/sound/alsa-configuration.rst
> @@ -2297,39 +2297,81 @@ skip_validation
> of the unit descriptor instead of a driver probe error, so that we
> can check its details.
> quirk_flags
> - Contains the bit flags for various device specific workarounds.
> - Applied to the corresponding card index.
> -
> - * bit 0: Skip reading sample rate for devices
> - * bit 1: Create Media Controller API entries
> - * bit 2: Allow alignment on audio sub-slot at transfer
> - * bit 3: Add length specifier to transfers
> - * bit 4: Start playback stream at first in implement feedback mode
> - * bit 5: Skip clock selector setup
> - * bit 6: Ignore errors from clock source search
> - * bit 7: Indicates ITF-USB DSD based DACs
> - * bit 8: Add a delay of 20ms at each control message handling
> - * bit 9: Add a delay of 1-2ms at each control message handling
> - * bit 10: Add a delay of 5-6ms at each control message handling
> - * bit 11: Add a delay of 50ms at each interface setup
> - * bit 12: Perform sample rate validations at probe
> - * bit 13: Disable runtime PM autosuspend
> - * bit 14: Ignore errors for mixer access
> - * bit 15: Support generic DSD raw U32_BE format
> - * bit 16: Set up the interface at first like UAC1
> - * bit 17: Apply the generic implicit feedback sync mode
> - * bit 18: Don't apply implicit feedback sync mode
> - * bit 19: Don't closed interface during setting sample rate
> - * bit 20: Force an interface reset whenever stopping & restarting
> - a stream
> - * bit 21: Do not set PCM rate (frequency) when only one rate is
> - available for the given endpoint.
> - * bit 22: Set the fixed resolution 16 for Mic Capture Volume
> - * bit 23: Set the fixed resolution 384 for Mic Capture Volume
> - * bit 24: Set minimum volume control value as mute for devices
> - where the lowest playback value represents muted state instead
> - of minimum audible volume
> - * bit 25: Be similar to bit 24 but for capture streams
> + The option provides a refined and flexible control for applying quirk
> + flags. It allows to specify the quirk flags for each device, and could
and may
or: and can
> + be modified dynamically via sysfs.
> + The old usage accepts an array of integers, each of which apply quirk
applies
> + flags on the device in the order of probing.
> + e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first
E.g.,
> + device, and share_media_device to the second device.
> + The new usage accepts a string in the format of
> + ``VID1:PID1:FLAGS1;VID2:PID2:FLAGS2;...``, where ``VIDx`` and ``PIDx``
> + specify the device, and ``FLAGSx`` specify the flags to be applied.
> + ``VIDx`` and ``PIDx`` are 4-digit hexadecimal numbers, and could be
s/could/may/
> + specified as ``*`` to match any value. ``FLAGSx`` could be a set of
s/could/may/
> + flags given by name, separated by ``|``, or a hexadecimal number
> + representing the bit flags. The available flag names are listed above.
s/above/below/ ?
> + An exclamation mark could be prefixed to a flag name to negate the flag.
s/could/may/
> + For example, ``1234:abcd:mixer_playback_min_mute|!ignore_ctl_error;*:*:0x01;``
What happens if the trailing (ending) ';' is omitted?
> + applies the ``mixer_playback_min_mute`` flag and clears the
> + ``ignore_ctl_error`` flag for the device 1234:abcd, and applies the
> + ``skip_sample_rate`` flag for all devices.
> +
> + * bit 0: ``get_sample_rate``
> + Skip reading sample rate for devices
get vs Skip is a little confusing.
> + * bit 1: ``share_media_device``
> + Create Media Controller API entries
> + * bit 2: ``align_transfer``
> + Allow alignment on audio sub-slot at transfer
> + * bit 3: ``tx_length``
> + Add length specifier to transfers
> + * bit 4: ``playback_first``
> + Start playback stream at first in implement feedback mode
> + * bit 5: ``skip_clock_selector``
> + Skip clock selector setup
> + * bit 6: ``ignore_clock_source``
> + Ignore errors from clock source search
> + * bit 7: ``itf_usb_dsd_dac``
> + Indicates ITF-USB DSD based DACs
DSD-based
> + * bit 8: ``ctl_msg_delay``
> + Add a delay of 20ms at each control message handling
> + * bit 9: ``ctl_msg_delay_1m``
> + Add a delay of 1-2ms at each control message handling
> + * bit 10: ``ctl_msg_delay_5m``
> + Add a delay of 5-6ms at each control message handling
> + * bit 11: ``iface_delay``
> + Add a delay of 50ms at each interface setup
> + * bit 12: ``validate_rates``
> + Perform sample rate validations at probe
> + * bit 13: ``disable_autosuspend``
> + Disable runtime PM autosuspend
> + * bit 14: ``ignore_ctl_error``
> + Ignore errors for mixer access
> + * bit 15: ``dsd_raw``
> + Support generic DSD raw U32_BE format
> + * bit 16: ``set_iface_first``
> + Set up the interface at first like UAC1
> + * bit 17: ``generic_implicit_fb``
> + Apply the generic implicit feedback sync mode
> + * bit 18: ``skip_implicit_fb``
> + Don't apply implicit feedback sync mode
> + * bit 19: ``iface_skip_close``
> + Don't closed interface during setting sample rate
close
> + * bit 20: ``force_iface_reset``
> + Force an interface reset whenever stopping & restarting a stream
> + * bit 21: ``fixed_rate``
> + Do not set PCM rate (frequency) when only one rate is available
> + for the given endpoint
> + * bit 22: ``mic_res_16``
> + Set the fixed resolution 16 for Mic Capture Volume
> + * bit 23: ``mic_res_384``
> + Set the fixed resolution 384 for Mic Capture Volume
> + * bit 24: ``mixer_playback_min_mute``
> + Set minimum volume control value as mute for devices where the
> + lowest playback value represents muted state instead of minimum
> + audible volume
> + * bit 25: ``mixer_capture_min_mute``
> + Be similar to bit 24 but for capture streams
Similar to
>
> This module supports multiple devices, autoprobe and hotplugging.
>
> Are all of these quirks used on various devices or are some of these
just implemented just in case they are needed in the future?thanks.
--
~Randy
^ permalink raw reply
* [PATCH 6/6] module: comment describing new codepath
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
added comment so future authors know about this codepath
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
kernel/module/main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 256e30259bcf..f4ce431163fa 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3220,6 +3220,11 @@ static int module_patient_check_exists(const char *name,
*/
if (old && old->state == MODULE_STATE_LIVE)
return -EEXIST;
+
+ /*
+ * Can occur if the module was forcefully unloaded after
+ * its initcall crashed.
+ */
return -EBUSY;
}
--
2.45.2
^ permalink raw reply related
* [PATCH 5/6] module: store and complete idempotent upon force unloading
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
Move idempotent definition up and add idempotent_complete declaration.
Add idempotent to struct load_info which gets passed into load_module
and then stored in the struct module.
run idempotent_complete after module is unloaded and give EBUSY
to any process waiting for the module to finish initializing
via finit_module.
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
kernel/module/internal.h | 3 +++
kernel/module/main.c | 29 +++++++++++++++++++++--------
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 8d74b0a21c82..43f537475859 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -89,6 +89,9 @@ struct load_info {
unsigned int vers_ext_crc;
unsigned int vers_ext_name;
} index;
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ struct idempotent* idempotent;
+#endif
};
enum mod_license {
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 217185dbc3c1..256e30259bcf 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -80,6 +80,17 @@ static void do_free_init(struct work_struct *w);
static DECLARE_WORK(init_free_wq, do_free_init);
static LLIST_HEAD(init_free_list);
+
+struct idempotent {
+ const void *cookie;
+ struct hlist_node entry;
+ struct completion complete;
+ int ret;
+};
+
+static int idempotent_complete(struct idempotent *u, int ret);
+
+
struct mod_tree_root mod_tree __cacheline_aligned = {
.addr_min = -1UL,
};
@@ -784,7 +795,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
!IS_ENABLED(CONFIG_MODULE_FORCE_UNLOAD))
) {
if (mod->state == MODULE_STATE_GOING)
- pr_debug("%s already dying\n", mod->name);
+ pr_debug("%s already dying\n", mod->name);
ret = -EBUSY;
goto out;
}
@@ -833,6 +844,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
strscpy(last_unloaded_module.name, mod->name);
strscpy(last_unloaded_module.taints, module_flags(mod, buf, false));
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ if (did_init_crash && mod->idempotent)
+ idempotent_complete(mod->idempotent, -EBUSY);
+#endif
+
free_module(mod);
/* someone could wait for the module in add_unformed_module() */
wake_up_all(&module_wq);
@@ -3591,12 +3607,6 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
return load_module(&info, uargs, 0);
}
-struct idempotent {
- const void *cookie;
- struct hlist_node entry;
- struct completion complete;
- int ret;
-};
#define IDEM_HASH_BITS 8
static struct hlist_head idem_hash[1 << IDEM_HASH_BITS];
@@ -3683,7 +3693,7 @@ static int idempotent_wait_for_completion(struct idempotent *u)
return ret;
}
-static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
+static int init_module_from_file(struct file *f, const char __user * uargs, int flags, struct idempotent *idempotent __maybe_unused)
{
struct load_info info = { };
void *buf = NULL;
@@ -3707,6 +3717,9 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
info.hdr = buf;
info.len = len;
}
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ info.idempotent = idempotent;
+#endif
return load_module(&info, uargs, flags);
}
--
2.45.2
^ permalink raw reply related
* [PATCH 4/6] module: move idempotent allocation to heap
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
struct idempotent is normally allocated on the stack.
If init crashes during a finit_module syscall and then the module is unloaded,
reloading the module causes issues. This is because the struct idempotent stored
in the list becomes stale after the crash.
Therefore idempotent is stored on the heap and placed inside the struct module
to be completed by delete_module.
I am open to the idea of only storing it in the heap when
CONFIG_MODULE_FORCE_UNLOAD is enabled; however, simple seemed better.
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
kernel/module/main.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2825ac177c62..217185dbc3c1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3650,6 +3650,8 @@ static int idempotent_complete(struct idempotent *u, int ret)
complete(&pos->complete);
}
spin_unlock(&idem_lock);
+
+ kfree(u);
return ret;
}
@@ -3666,13 +3668,19 @@ static int idempotent_complete(struct idempotent *u, int ret)
*/
static int idempotent_wait_for_completion(struct idempotent *u)
{
+ int ret;
+
if (wait_for_completion_interruptible(&u->complete)) {
spin_lock(&idem_lock);
if (!hlist_unhashed(&u->entry))
hlist_del(&u->entry);
spin_unlock(&idem_lock);
}
- return u->ret;
+ ret = u->ret;
+
+ kfree(u);
+
+ return ret;
}
static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
@@ -3705,21 +3713,26 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
{
- struct idempotent idem;
+ struct idempotent *idem;
+
+ idem = kmalloc(sizeof(*idem), GFP_KERNEL);
+ if (!idem)
+ return -ENOMEM;
+
if (!(f->f_mode & FMODE_READ))
return -EBADF;
/* Are we the winners of the race and get to do this? */
- if (!idempotent(&idem, file_inode(f))) {
- int ret = init_module_from_file(f, uargs, flags);
- return idempotent_complete(&idem, ret);
+ if (!idempotent(idem, file_inode(f))) {
+ int ret = init_module_from_file(f, uargs, flags, idem);
+ return idempotent_complete(idem, ret);
}
/*
* Somebody else won the race and is loading the module.
*/
- return idempotent_wait_for_completion(&idem);
+ return idempotent_wait_for_completion(idem);
}
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
--
2.45.2
^ permalink raw reply related
* [PATCH 3/6] module: move freeinit allocation to avoid memory leak
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
move freeinit allocation after do_one_initcall in case do_one_initcall crashes.
Otherwise, freeinit would leak memory after every initalization of a crashed module.
I could not find a reason for why freeinit allocation happened before do_one_initcall.
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
kernel/module/main.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2277c53aef2e..2825ac177c62 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3021,21 +3021,13 @@ static noinline int do_init_module(struct module *mod)
}
#endif
- freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
- if (!freeinit) {
- ret = -ENOMEM;
- goto fail;
- }
- freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
- freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
- freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
do_mod_ctors(mod);
/* Start the module */
if (mod->init != NULL)
ret = do_one_initcall(mod->init);
if (ret < 0) {
- goto fail_free_freeinit;
+ goto fail;
}
if (ret > 0) {
pr_warn("%s: '%s'->init suspiciously returned %d, it should "
@@ -3045,6 +3037,14 @@ static noinline int do_init_module(struct module *mod)
dump_stack();
}
+ freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL);
+ if (!freeinit) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+ freeinit->init_text = mod->mem[MOD_INIT_TEXT].base;
+ freeinit->init_data = mod->mem[MOD_INIT_DATA].base;
+ freeinit->init_rodata = mod->mem[MOD_INIT_RODATA].base;
/* Now it's a first class citizen! */
mod->state = MODULE_STATE_LIVE;
blocking_notifier_call_chain(&module_notify_list,
@@ -3123,8 +3123,6 @@ static noinline int do_init_module(struct module *mod)
return 0;
-fail_free_freeinit:
- kfree(freeinit);
fail:
/* Try to protect us from buggy refcounters. */
mod->state = MODULE_STATE_GOING;
--
2.45.2
^ permalink raw reply related
* [PATCH 2/6] module: detect if init crashed and unload
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
Store idempotent and init_pid in struct module.
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
kernel/module/main.c | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 413ac6ea3702..2277c53aef2e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -752,6 +752,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
char name[MODULE_NAME_LEN];
char buf[MODULE_FLAGS_BUF_SIZE];
int ret, forced = 0;
+ bool did_init_crash __maybe_unused = false;
if (!capable(CAP_SYS_MODULE) || modules_disabled)
return -EPERM;
@@ -778,8 +779,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
}
/* Doing init or already dying? */
- if (mod->state != MODULE_STATE_LIVE) {
- /* FIXME: if (force), slam module count damn the torpedoes */
+ if (mod->state == MODULE_STATE_GOING ||
+ (mod->state != MODULE_STATE_LIVE &&
+ !IS_ENABLED(CONFIG_MODULE_FORCE_UNLOAD))
+ ) {
+ if (mod->state == MODULE_STATE_GOING)
pr_debug("%s already dying\n", mod->name);
ret = -EBUSY;
goto out;
@@ -795,6 +799,21 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
}
}
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ if (mod->state == MODULE_STATE_COMING) {
+ struct task_struct *init_process = get_pid_task(mod->init_pid, PIDTYPE_PID);
+
+ /* Did the init process die? */
+ if (init_process) {
+ put_task_struct(init_process);
+ ret = -EBUSY;
+ goto out;
+ } else {
+ did_init_crash = true;
+ }
+ }
+#endif
+
ret = try_stop_module(mod, flags, &forced);
if (ret != 0)
goto out;
@@ -1380,6 +1399,10 @@ static void free_module(struct module *mod)
mod->state = MODULE_STATE_UNFORMED;
mutex_unlock(&module_mutex);
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ if (mod->init_pid)
+ put_pid(mod->init_pid);
+#endif
/* Arch-specific cleanup. */
module_arch_cleanup(mod);
@@ -3044,6 +3067,11 @@ static noinline int do_init_module(struct module *mod)
ftrace_free_mem(mod, mod->mem[MOD_INIT_TEXT].base,
mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
mutex_lock(&module_mutex);
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ put_pid(mod->init_pid);
+ mod->init_pid = NULL;
+ mod->idempotent = NULL;
+#endif
/* Drop initial reference. */
module_put(mod);
trim_init_extable(mod);
@@ -3474,6 +3502,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
if (codetag_load_module(mod))
goto sysfs_cleanup;
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ mod->init_pid = get_pid(task_pid(current));
+ mod->idempotent = info->idempotent;
+#endif
/* Get rid of temporary copy. */
free_copy(info, flags);
--
2.45.2
^ permalink raw reply related
* [PATCH 1/6] module: store init_pid and idempotent in module
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
include/linux/module.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/linux/module.h b/include/linux/module.h
index 5faa1fb1f4b4..5ac5f4992fe8 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -599,6 +599,10 @@ struct module {
#ifdef CONFIG_DYNAMIC_DEBUG_CORE
struct _ddebug_info dyndbg_info;
#endif
+#ifdef CONFIG_MODULE_FORCE_UNLOAD
+ struct pid *init_pid;
+ struct idempotent *idempotent;
+#endif
} ____cacheline_aligned __randomize_layout;
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}
--
2.45.2
^ permalink raw reply related
* [PATCH 0/6] module: enable force unloading of modules that have crashed during init
From: julian-lagattuta @ 2025-09-18 20:11 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu
Cc: Sami Tolvanen, Daniel Gomez, linux-modules, linux-kernel,
julian-lagattuta
Running a module that encounters a fatal error during the initcall leaves
the module in a state where it cannot be forcefully unloaded since it is
"being used" despite there being no reason it couldn't be unloaded.
This means that unloading the crashed module requires rebooting.
This patch allows modules that have crashed during initialization to be
forcefully unloaded with CONFIG_MODULE_FORCE_UNLOAD enabled.
Here are the costs:
- 2 extra pointers stored in struct module (if CONFIG_MODULE_FORCE_UNLOAD is enabled)
- struct idempotent is allocated on heap instead of stack (regardless of config)
I had to make a design decision for cases where another (f)init_module is
waiting for the crashed module to finish and delete_module is called on it.
Here is an example of the behavior my patch causes:
> insmod crash.ko # insmod calls finit_module and crash.ko runs 0/0 in init
Segmentation Fault
> insmod crash.ko & # insmod will hang forever since it waits for cash.ko init to finish
> rmmod -f crash
[1]+ Exit 1 insmod crash.ko
insmod: ERROR: could not insert module crash.ko: Device or resource busy
Here, anyone waiting for init to finish will receive -EBUSY upon removal.
This is true for finit_module and init_module syscalls.
I chose -EBUSY since it means I don't need to modify module_patient_check_exists.
error -ECANCELED might work better
P.S. This is my first patch so I'm new to this.
Signed-off-by: julian-lagattuta <julian.lagattuta@gmail.com>
---
julian-lagattuta (6):
module: store init_pid and idempotent in module
module: detect if init crashed and unload
module: move freeinit allocation to avoid memory leak
module: move idempotent allocation to heap
module: store and complete idempotent upon force unloading
module: comment to describe a new codepath
include/linux/module.h | 4 ++
kernel/module/internal.h | 3 ++
kernel/module/main.c | 112 +++++++++++++++++++++++++++++----------
3 files changed, 92 insertions(+), 27 deletions(-)
--
2.45.2
^ permalink raw reply
* Re: [PATCH v8 0/8] Add generated modalias to modules.builtin.modinfo
From: Nathan Chancellor @ 2025-09-18 17:35 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nicolas Schier, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, linux-kernel, linux-modules, linux-kbuild
In-Reply-To: <cover.1758182101.git.legion@kernel.org>
Hi Alexey,
On Thu, Sep 18, 2025 at 10:05:44AM +0200, Alexey Gladkov wrote:
> The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> get information about builtin modules. Among other information about the module,
> information about module aliases is stored. This is very important to determine
> that a particular modalias will be handled by a module that is inside the
> kernel.
>
> There are several mechanisms for creating modalias for modules:
>
> The first is to explicitly specify the MODULE_ALIAS of the macro. In this case,
> the aliases go into the '.modinfo' section of the module if it is compiled
> separately or into vmlinux.o if it is builtin into the kernel.
>
> The second is the use of MODULE_DEVICE_TABLE followed by the use of the
> modpost utility. In this case, vmlinux.o no longer has this information and
> does not get it into modules.builtin.modinfo.
>
> For example:
>
> $ modinfo pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> modinfo: ERROR: Module pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30 not found.
>
> $ modinfo xhci_pci
> name: xhci_pci
> filename: (builtin)
> license: GPL
> file: drivers/usb/host/xhci-pci
> description: xHCI PCI Host Controller Driver
>
> The builtin module is missing alias "pci:v*d*sv*sd*bc0Csc03i30*" which will be
> generated by modpost if the module is built separately.
>
> To fix this it is necessary to add the generated by modpost modalias to
> modules.builtin.modinfo. Fortunately modpost already generates .vmlinux.export.c
> for exported symbols. It is possible to add `.modinfo` for builtin modules and
> modify the build system so that `.modinfo` section is extracted from the
> intermediate vmlinux after modpost is executed.
This seems good to me now. I have tentatively merged this into
kbuild-next so that it can get some more soak testing in -next, as I am
going to be paying less attention to email starting on Sunday for a week
or so, so I would like to be able to deal with regressions promptly.
https://git.kernel.org/kbuild/l/kbuild-next
Don't consider the commit hashes stable just yet, as I would like to
give Nicolas the chance to provide tags if he would like but I did not
want to wait for the soak testing.
Cheers,
Nathan
^ permalink raw reply
* [PATCH v4 5/5] ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae
In-Reply-To: <20250918-sound-v4-0-82cf8123d61c@uniontech.com>
From: Cryolitia PukNgae <cryolitia@uniontech.com>
Just briefly described about the option.
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
Documentation/sound/alsa-configuration.rst | 108 ++++++++++++++++++++---------
1 file changed, 75 insertions(+), 33 deletions(-)
diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst
index a2fb8ed251dd0294e7a62209ca15d5c32c6adfae..efffe3d534beeddcb6a47ac27a24defb6879f534 100644
--- a/Documentation/sound/alsa-configuration.rst
+++ b/Documentation/sound/alsa-configuration.rst
@@ -2297,39 +2297,81 @@ skip_validation
of the unit descriptor instead of a driver probe error, so that we
can check its details.
quirk_flags
- Contains the bit flags for various device specific workarounds.
- Applied to the corresponding card index.
-
- * bit 0: Skip reading sample rate for devices
- * bit 1: Create Media Controller API entries
- * bit 2: Allow alignment on audio sub-slot at transfer
- * bit 3: Add length specifier to transfers
- * bit 4: Start playback stream at first in implement feedback mode
- * bit 5: Skip clock selector setup
- * bit 6: Ignore errors from clock source search
- * bit 7: Indicates ITF-USB DSD based DACs
- * bit 8: Add a delay of 20ms at each control message handling
- * bit 9: Add a delay of 1-2ms at each control message handling
- * bit 10: Add a delay of 5-6ms at each control message handling
- * bit 11: Add a delay of 50ms at each interface setup
- * bit 12: Perform sample rate validations at probe
- * bit 13: Disable runtime PM autosuspend
- * bit 14: Ignore errors for mixer access
- * bit 15: Support generic DSD raw U32_BE format
- * bit 16: Set up the interface at first like UAC1
- * bit 17: Apply the generic implicit feedback sync mode
- * bit 18: Don't apply implicit feedback sync mode
- * bit 19: Don't closed interface during setting sample rate
- * bit 20: Force an interface reset whenever stopping & restarting
- a stream
- * bit 21: Do not set PCM rate (frequency) when only one rate is
- available for the given endpoint.
- * bit 22: Set the fixed resolution 16 for Mic Capture Volume
- * bit 23: Set the fixed resolution 384 for Mic Capture Volume
- * bit 24: Set minimum volume control value as mute for devices
- where the lowest playback value represents muted state instead
- of minimum audible volume
- * bit 25: Be similar to bit 24 but for capture streams
+ The option provides a refined and flexible control for applying quirk
+ flags. It allows to specify the quirk flags for each device, and could
+ be modified dynamically via sysfs.
+ The old usage accepts an array of integers, each of which apply quirk
+ flags on the device in the order of probing.
+ e.g. ``quirk_flags=0x01,0x02`` applies get_sample_rate to the first
+ device, and share_media_device to the second device.
+ The new usage accepts a string in the format of
+ ``VID1:PID1:FLAGS1;VID2:PID2:FLAGS2;...``, where ``VIDx`` and ``PIDx``
+ specify the device, and ``FLAGSx`` specify the flags to be applied.
+ ``VIDx`` and ``PIDx`` are 4-digit hexadecimal numbers, and could be
+ specified as ``*`` to match any value. ``FLAGSx`` could be a set of
+ flags given by name, separated by ``|``, or a hexadecimal number
+ representing the bit flags. The available flag names are listed above.
+ An exclamation mark could be prefixed to a flag name to negate the flag.
+ For example, ``1234:abcd:mixer_playback_min_mute|!ignore_ctl_error;*:*:0x01;``
+ applies the ``mixer_playback_min_mute`` flag and clears the
+ ``ignore_ctl_error`` flag for the device 1234:abcd, and applies the
+ ``skip_sample_rate`` flag for all devices.
+
+ * bit 0: ``get_sample_rate``
+ Skip reading sample rate for devices
+ * bit 1: ``share_media_device``
+ Create Media Controller API entries
+ * bit 2: ``align_transfer``
+ Allow alignment on audio sub-slot at transfer
+ * bit 3: ``tx_length``
+ Add length specifier to transfers
+ * bit 4: ``playback_first``
+ Start playback stream at first in implement feedback mode
+ * bit 5: ``skip_clock_selector``
+ Skip clock selector setup
+ * bit 6: ``ignore_clock_source``
+ Ignore errors from clock source search
+ * bit 7: ``itf_usb_dsd_dac``
+ Indicates ITF-USB DSD based DACs
+ * bit 8: ``ctl_msg_delay``
+ Add a delay of 20ms at each control message handling
+ * bit 9: ``ctl_msg_delay_1m``
+ Add a delay of 1-2ms at each control message handling
+ * bit 10: ``ctl_msg_delay_5m``
+ Add a delay of 5-6ms at each control message handling
+ * bit 11: ``iface_delay``
+ Add a delay of 50ms at each interface setup
+ * bit 12: ``validate_rates``
+ Perform sample rate validations at probe
+ * bit 13: ``disable_autosuspend``
+ Disable runtime PM autosuspend
+ * bit 14: ``ignore_ctl_error``
+ Ignore errors for mixer access
+ * bit 15: ``dsd_raw``
+ Support generic DSD raw U32_BE format
+ * bit 16: ``set_iface_first``
+ Set up the interface at first like UAC1
+ * bit 17: ``generic_implicit_fb``
+ Apply the generic implicit feedback sync mode
+ * bit 18: ``skip_implicit_fb``
+ Don't apply implicit feedback sync mode
+ * bit 19: ``iface_skip_close``
+ Don't closed interface during setting sample rate
+ * bit 20: ``force_iface_reset``
+ Force an interface reset whenever stopping & restarting a stream
+ * bit 21: ``fixed_rate``
+ Do not set PCM rate (frequency) when only one rate is available
+ for the given endpoint
+ * bit 22: ``mic_res_16``
+ Set the fixed resolution 16 for Mic Capture Volume
+ * bit 23: ``mic_res_384``
+ Set the fixed resolution 384 for Mic Capture Volume
+ * bit 24: ``mixer_playback_min_mute``
+ Set minimum volume control value as mute for devices where the
+ lowest playback value represents muted state instead of minimum
+ audible volume
+ * bit 25: ``mixer_capture_min_mute``
+ Be similar to bit 24 but for capture streams
This module supports multiple devices, autoprobe and hotplugging.
--
2.51.0
^ permalink raw reply related
* [PATCH v4 4/5] ALSA: usb-audio: make param quirk_flags change-able in runtime
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae, Takashi Iwai
In-Reply-To: <20250918-sound-v4-0-82cf8123d61c@uniontech.com>
From: Cryolitia PukNgae <cryolitia@uniontech.com>
Developers now can change it during sysfs, without rebooting, for
debugging new buggy devices.
Co-developed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
sound/usb/card.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++--
sound/usb/quirks.c | 9 +++++++--
sound/usb/usbaudio.h | 2 ++
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 5837677effa1787ef9d7d02714c3ae43b1a8bc79..fd99df5949df826d97b3d2bc6d3137923ab4295d 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -103,13 +103,65 @@ module_param_array(delayed_register, charp, NULL, 0444);
MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
module_param_array(implicit_fb, bool, NULL, 0444);
MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
-module_param_array(quirk_flags, charp, NULL, 0444);
-MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444);
MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no).");
+/* protects quirk_flags */
+DEFINE_MUTEX(quirk_flags_mutex);
+
+static int quirk_flags_param_set(const char *value,
+ const struct kernel_param *kp)
+{
+ int err;
+
+ mutex_lock(&quirk_flags_mutex);
+
+ memset(quirk_flags, 0, sizeof(quirk_flags));
+ err = param_array_set(value, kp);
+
+ mutex_unlock(&quirk_flags_mutex);
+
+ return err;
+}
+
+static int quirk_flags_param_get(char *buffer, const struct kernel_param *kp)
+{
+ int err;
+
+ mutex_lock(&quirk_flags_mutex);
+ err = param_array_get(buffer, kp);
+ mutex_unlock(&quirk_flags_mutex);
+
+ return err;
+}
+
+static void quirk_flags_param_free(void *arg)
+{
+ mutex_lock(&quirk_flags_mutex);
+ param_array_free(arg);
+ mutex_unlock(&quirk_flags_mutex);
+}
+
+static const struct kernel_param_ops quirk_flags_param_ops = {
+ .set = quirk_flags_param_set,
+ .get = quirk_flags_param_get,
+ .free = quirk_flags_param_free,
+};
+
+static struct kparam_array quirk_flags_param_array = {
+ .max = SNDRV_CARDS,
+ .elemsize = sizeof(char *),
+ .num = NULL,
+ .ops = ¶m_ops_charp,
+ .elem = &quirk_flags,
+};
+
+device_param_cb(quirk_flags, &quirk_flags_param_ops, &quirk_flags_param_array,
+ 0644);
+MODULE_PARM_DESC(quirk_flags, "Add/modify USB audio quirks");
+
/*
* we keep the snd_usb_audio_t instances by ourselves for merging
* the all interfaces on the same card as one sound device.
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 4dc2464133e934e48b1fa613884a8a0ebdaff91d..ba9ea94399e5a6e7e1711b2a9148c90f991f05cd 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2548,6 +2548,8 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
u16 vid, pid;
size_t i;
+ mutex_lock(&quirk_flags_mutex);
+
/* old style option found: the position-based integer value */
if (quirk_flags[idx] &&
!kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
@@ -2556,7 +2558,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
chip->quirk_flags, idx,
USB_ID_VENDOR(chip->usb_id),
USB_ID_PRODUCT(chip->usb_id));
- return;
+ goto unlock;
}
/* take the default quirk from the quirk table */
@@ -2571,7 +2573,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
if (!val) {
pr_err("snd_usb_audio: Error allocating memory while parsing quirk_flags\n");
- return;
+ goto unlock;
}
for (p = val; p && *p;) {
@@ -2653,4 +2655,7 @@ void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
kfree(val);
}
+
+unlock:
+ mutex_unlock(&quirk_flags_mutex);
}
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 73564cd68ebf101291440d0171eb81c220c6f5e2..40551946c20df8c17d306fddd8295ca3a2bfa702 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -168,6 +168,8 @@ extern char *quirk_flags[SNDRV_CARDS];
extern bool snd_usb_use_vmalloc;
extern bool snd_usb_skip_validation;
+extern struct mutex quirk_flags_mutex;
+
/*
* Driver behavior quirk flags, stored in chip->quirk_flags
*
--
2.51.0
^ permalink raw reply related
* [PATCH v4 3/5] ALSA: usb-audio: improve module param quirk_flags
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae, Takashi Iwai
In-Reply-To: <20250918-sound-v4-0-82cf8123d61c@uniontech.com>
From: Cryolitia PukNgae <cryolitia@uniontech.com>
For apply and unapply quirk flags more flexibly though param
Co-developed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
sound/usb/card.c | 9 ++--
sound/usb/quirks.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++-
sound/usb/quirks.h | 3 +-
sound/usb/usbaudio.h | 3 ++
4 files changed, 126 insertions(+), 8 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 0265206a8e8cf31133e8463c98fe0497d8ace89e..5837677effa1787ef9d7d02714c3ae43b1a8bc79 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -73,8 +73,8 @@ static bool lowlatency = true;
static char *quirk_alias[SNDRV_CARDS];
static char *delayed_register[SNDRV_CARDS];
static bool implicit_fb[SNDRV_CARDS];
-static unsigned int quirk_flags[SNDRV_CARDS];
+char *quirk_flags[SNDRV_CARDS];
bool snd_usb_use_vmalloc = true;
bool snd_usb_skip_validation;
@@ -103,7 +103,7 @@ module_param_array(delayed_register, charp, NULL, 0444);
MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4.");
module_param_array(implicit_fb, bool, NULL, 0444);
MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode.");
-module_param_array(quirk_flags, uint, NULL, 0444);
+module_param_array(quirk_flags, charp, NULL, 0444);
MODULE_PARM_DESC(quirk_flags, "Driver quirk bit flags.");
module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444);
MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes).");
@@ -750,10 +750,7 @@ static int snd_usb_audio_create(struct usb_interface *intf,
INIT_LIST_HEAD(&chip->midi_v2_list);
INIT_LIST_HEAD(&chip->mixer_list);
- if (quirk_flags[idx])
- chip->quirk_flags = quirk_flags[idx];
- else
- snd_usb_init_quirk_flags(chip);
+ snd_usb_init_quirk_flags(idx, chip);
card->private_free = snd_usb_audio_free;
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 94854f352b1702b491e1bf3c8b769f7088e03976..4dc2464133e934e48b1fa613884a8a0ebdaff91d 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2502,7 +2502,7 @@ u32 snd_usb_quirk_flags_from_name(char *name)
return flag;
}
-void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
+void snd_usb_init_quirk_flags_table(struct snd_usb_audio *chip)
{
const struct usb_audio_quirk_flags_table *p;
@@ -2537,3 +2537,120 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
}
}
}
+
+void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
+{
+ u16 chip_vid = USB_ID_VENDOR(chip->usb_id);
+ u16 chip_pid = USB_ID_PRODUCT(chip->usb_id);
+ u32 mask_flags, unmask_flags, bit;
+ char *val, *p, *field, *flag;
+ bool is_unmask;
+ u16 vid, pid;
+ size_t i;
+
+ /* old style option found: the position-based integer value */
+ if (quirk_flags[idx] &&
+ !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
+ usb_audio_dbg(chip,
+ "Set quirk flags 0x%x from param based on position %d for device %04x:%04x\n",
+ chip->quirk_flags, idx,
+ USB_ID_VENDOR(chip->usb_id),
+ USB_ID_PRODUCT(chip->usb_id));
+ return;
+ }
+
+ /* take the default quirk from the quirk table */
+ snd_usb_init_quirk_flags_table(chip);
+
+ /* add or correct quirk bits from options */
+ for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
+ if (!quirk_flags[i] || !*quirk_flags[i])
+ break;
+
+ val = kstrdup(quirk_flags[i], GFP_KERNEL);
+
+ if (!val) {
+ pr_err("snd_usb_audio: Error allocating memory while parsing quirk_flags\n");
+ return;
+ }
+
+ for (p = val; p && *p;) {
+ /* Each entry consists of VID:PID:flags */
+ field = strsep(&p, ":");
+ if (!field)
+ break;
+
+ if (strcmp(field, "*") == 0)
+ vid = 0;
+ else if (kstrtou16(field, 16, &vid))
+ break;
+
+ field = strsep(&p, ":");
+ if (!field)
+ break;
+
+ if (strcmp(field, "*") == 0)
+ pid = 0;
+ else if (kstrtou16(field, 16, &pid))
+ break;
+
+ field = strsep(&p, ";");
+ if (!field || !*field)
+ break;
+
+ if ((vid != 0 && vid != chip_vid) ||
+ (pid != 0 && pid != chip_pid))
+ continue;
+
+ /* Collect the flags */
+ mask_flags = 0;
+ unmask_flags = 0;
+ while (field && *field) {
+ flag = strsep(&field, "|");
+
+ if (!flag)
+ break;
+
+ if (*flag == '!') {
+ is_unmask = true;
+ flag++;
+ } else {
+ is_unmask = false;
+ }
+
+ if (!kstrtou32(flag, 16, &bit)) {
+ if (is_unmask)
+ unmask_flags |= bit;
+ else
+ mask_flags |= bit;
+
+ break;
+ }
+
+ bit = snd_usb_quirk_flags_from_name(flag);
+
+ if (bit) {
+ if (is_unmask)
+ unmask_flags |= bit;
+ else
+ mask_flags |= bit;
+ } else {
+ pr_warn("snd_usb_audio: unknown flag %s while parsing param quirk_flags\n",
+ field);
+ }
+ }
+
+ chip->quirk_flags &= ~unmask_flags;
+ chip->quirk_flags |= mask_flags;
+ usb_audio_dbg(chip,
+ "Set quirk flags (mask 0x%x, unmask 0x%x, finally 0x%x) from param for device %04x:%04x\n",
+ mask_flags,
+ unmask_flags,
+ chip->quirk_flags,
+ USB_ID_VENDOR(chip->usb_id),
+ USB_ID_PRODUCT(chip->usb_id));
+ }
+
+ kfree(val);
+ }
+}
diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h
index bd5baf2b193a1985f3a0e52bf4a77ca741364769..0dc8c04afeffe76e1219c71a8fe2e4b66624b48f 100644
--- a/sound/usb/quirks.h
+++ b/sound/usb/quirks.h
@@ -48,7 +48,8 @@ void snd_usb_audioformat_attributes_quirk(struct snd_usb_audio *chip,
struct audioformat *fp,
int stream);
-void snd_usb_init_quirk_flags(struct snd_usb_audio *chip);
+void snd_usb_init_quirk_flags_table(struct snd_usb_audio *chip);
+void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip);
const char *snd_usb_quirk_flag_find_name(unsigned long flag);
u32 snd_usb_quirk_flags_from_name(char *name);
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 0a22cb4a02344b2dcf4009c560a759f2da25ca67..73564cd68ebf101291440d0171eb81c220c6f5e2 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -7,6 +7,8 @@
* Copyright (c) 2002 by Takashi Iwai <tiwai@suse.de>
*/
+ #include <sound/core.h>
+
/* handling of USB vendor/product ID pairs as 32-bit numbers */
#define USB_ID(vendor, product) (((unsigned int)(vendor) << 16) | (product))
#define USB_ID_VENDOR(id) ((id) >> 16)
@@ -162,6 +164,7 @@ DEFINE_CLASS(snd_usb_lock, struct __snd_usb_lock,
__snd_usb_unlock_shutdown(&(_T)), __snd_usb_lock_shutdown(chip),
struct snd_usb_audio *chip)
+extern char *quirk_flags[SNDRV_CARDS];
extern bool snd_usb_use_vmalloc;
extern bool snd_usb_skip_validation;
--
2.51.0
^ permalink raw reply related
* [PATCH v4 0/5] ALSA: usb-audio: add module param device_quirk_flags
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae, Takashi Iwai
As an implementation of what has been discussed previously[1].
1. https://lore.kernel.org/all/87h5xm5g7f.wl-tiwai@suse.de/
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
Changes in v4:
- Split basic parse and dynamic change
- Drop usage of linked list
- Link to v3: https://lore.kernel.org/r/20250917-sound-v3-0-92ebe9472a0a@uniontech.com
Changes in v3:
- Instead of a new param, improve the existed one.
- Link to v2: https://lore.kernel.org/r/20250912-sound-v2-0-01ea3d279f4b@uniontech.com
Changes in v2:
- Cleaned up some internal rebase confusion, sorry for that
- Link to v1: https://lore.kernel.org/r/20250912-sound-v1-0-cc9cfd9f2d01@uniontech.com
---
Cryolitia PukNgae (5):
ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
param: export param_array related functions
ALSA: usb-audio: improve module param quirk_flags
ALSA: usb-audio: make param quirk_flags change-able in runtime
ALSA: doc: add docs about improved quirk_flags in snd-usb-audio
Documentation/sound/alsa-configuration.rst | 108 ++++++++++-----
include/linux/moduleparam.h | 3 +
kernel/params.c | 9 +-
sound/usb/card.c | 63 ++++++++-
sound/usb/quirks.c | 206 ++++++++++++++++++++++++++++-
sound/usb/quirks.h | 6 +-
sound/usb/usbaudio.h | 6 +
7 files changed, 352 insertions(+), 49 deletions(-)
---
base-commit: 4c421c40c8b30ab7aae1edc7f7e294fcd33fc186
change-id: 20250910-sound-a91c86c92dba
Best regards,
--
Cryolitia PukNgae <cryolitia@uniontech.com>
^ permalink raw reply
* [PATCH v4 1/5] ALSA: usb-audio: add two-way convert between name and bit for QUIRK_FLAG_*
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae
In-Reply-To: <20250918-sound-v4-0-82cf8123d61c@uniontech.com>
From: Cryolitia PukNgae <cryolitia@uniontech.com>
Also improve debug logs for applied quirks
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
sound/usb/quirks.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++---
sound/usb/quirks.h | 3 ++
sound/usb/usbaudio.h | 1 +
3 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index d736a4750356597bfb0f9d5ab01cdaeaac0f907c..94854f352b1702b491e1bf3c8b769f7088e03976 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -2446,6 +2446,62 @@ static const struct usb_audio_quirk_flags_table quirk_flags_table[] = {
{} /* terminator */
};
+static const char *const snd_usb_audio_quirk_flag_names[] = {
+ "get_sample_rate",
+ "share_media_device",
+ "align_transfer",
+ "tx_length",
+ "playback_first",
+ "skip_clock_selector",
+ "ignore_clock_source",
+ "itf_usb_dsd_dac",
+ "ctl_msg_delay",
+ "ctl_msg_delay_1m",
+ "ctl_msg_delay_5m",
+ "iface_delay",
+ "validate_rates",
+ "disable_autosuspend",
+ "ignore_ctl_error",
+ "dsd_raw",
+ "set_iface_first",
+ "generic_implicit_fb",
+ "skip_implicit_fb",
+ "iface_skip_close",
+ "force_iface_reset",
+ "fixed_rate",
+ "mic_res_16",
+ "mic_res_384",
+ "mixer_playback_min_mute",
+ "mixer_capture_min_mute",
+ NULL
+};
+
+const char *snd_usb_quirk_flag_find_name(unsigned long index)
+{
+ if (index >= ARRAY_SIZE(snd_usb_audio_quirk_flag_names))
+ return NULL;
+
+ return snd_usb_audio_quirk_flag_names[index];
+}
+
+u32 snd_usb_quirk_flags_from_name(char *name)
+{
+ u32 flag = 0;
+ u32 i;
+
+ if (!name || !*name)
+ return 0;
+
+ for (i = 0; snd_usb_audio_quirk_flag_names[i]; i++) {
+ if (strcmp(name, snd_usb_audio_quirk_flag_names[i]) == 0) {
+ flag = (1U << i);
+ break;
+ }
+ }
+
+ return flag;
+}
+
void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
{
const struct usb_audio_quirk_flags_table *p;
@@ -2454,10 +2510,28 @@ void snd_usb_init_quirk_flags(struct snd_usb_audio *chip)
if (chip->usb_id == p->id ||
(!USB_ID_PRODUCT(p->id) &&
USB_ID_VENDOR(chip->usb_id) == USB_ID_VENDOR(p->id))) {
- usb_audio_dbg(chip,
- "Set quirk_flags 0x%x for device %04x:%04x\n",
- p->flags, USB_ID_VENDOR(chip->usb_id),
- USB_ID_PRODUCT(chip->usb_id));
+ unsigned long flags = p->flags;
+ unsigned long bit;
+
+ for_each_set_bit(bit, &flags,
+ BYTES_TO_BITS(sizeof(p->flags))) {
+ const char *name =
+ snd_usb_audio_quirk_flag_names[bit];
+
+ if (name)
+ usb_audio_dbg(chip,
+ "Set quirk flag %s for device %04x:%04x\n",
+ name,
+ USB_ID_VENDOR(chip->usb_id),
+ USB_ID_PRODUCT(chip->usb_id));
+ else
+ usb_audio_warn(chip,
+ "Set unknown quirk flag 0x%lx for device %04x:%04x\n",
+ bit,
+ USB_ID_VENDOR(chip->usb_id),
+ USB_ID_PRODUCT(chip->usb_id));
+ }
+
chip->quirk_flags |= p->flags;
return;
}
diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h
index f9bfd5ac7bab01717de3a76227482a128bf73165..bd5baf2b193a1985f3a0e52bf4a77ca741364769 100644
--- a/sound/usb/quirks.h
+++ b/sound/usb/quirks.h
@@ -50,4 +50,7 @@ void snd_usb_audioformat_attributes_quirk(struct snd_usb_audio *chip,
void snd_usb_init_quirk_flags(struct snd_usb_audio *chip);
+const char *snd_usb_quirk_flag_find_name(unsigned long flag);
+u32 snd_usb_quirk_flags_from_name(char *name);
+
#endif /* __USBAUDIO_QUIRKS_H */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 30b5102e3caed01eeb86d0075c41338104c58950..0a22cb4a02344b2dcf4009c560a759f2da25ca67 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -252,5 +252,6 @@ extern bool snd_usb_skip_validation;
#define QUIRK_FLAG_MIC_RES_384 (1U << 23)
#define QUIRK_FLAG_MIXER_PLAYBACK_MIN_MUTE (1U << 24)
#define QUIRK_FLAG_MIXER_CAPTURE_MIN_MUTE (1U << 25)
+/* Please also edit snd_usb_audio_quirk_flag_names */
#endif /* __USBAUDIO_H */
--
2.51.0
^ permalink raw reply related
* [PATCH v4 2/5] param: export param_array related functions
From: Cryolitia PukNgae via B4 Relay @ 2025-09-18 9:24 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai, Jonathan Corbet, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-sound, linux-usb, linux-kernel, linux-doc, Mingcong Bai,
Kexy Biscuit, Nie Cheng, Zhan Jun, Feng Yuan, qaqland, kernel,
linux-modules, Cryolitia PukNgae
In-Reply-To: <20250918-sound-v4-0-82cf8123d61c@uniontech.com>
From: Cryolitia PukNgae <cryolitia@uniontech.com>
- int param_array_set(const char *val, const struct kernel_param *kp);
- int param_array_get(char *buffer, const struct kernel_param *kp);
- void param_array_free(void *arg);
It would be helpful for the new module param we designed in
snd_usb_audio, in order to run additional custom codes when params
are set in runtime, and re-use the extisted codes in param.c
Signed-off-by: Cryolitia PukNgae <cryolitia@uniontech.com>
---
include/linux/moduleparam.h | 3 +++
kernel/params.c | 9 ++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 3a25122d83e2802e6e6a1475a52816251498b26a..4ef09ad2004789855bd21783029c653fac94b9dd 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -593,6 +593,9 @@ enum hwparam_type {
extern const struct kernel_param_ops param_array_ops;
+extern int param_array_set(const char *val, const struct kernel_param *kp);
+extern int param_array_get(char *buffer, const struct kernel_param *kp);
+extern void param_array_free(void *arg);
extern const struct kernel_param_ops param_ops_string;
extern int param_set_copystring(const char *val, const struct kernel_param *);
diff --git a/kernel/params.c b/kernel/params.c
index b96cfd693c9968012d42acb85611fee1acd47790..a936e018a1c6d0bf2b6b4566f80751840366f652 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -462,7 +462,7 @@ static int param_array(struct module *mod,
return 0;
}
-static int param_array_set(const char *val, const struct kernel_param *kp)
+int param_array_set(const char *val, const struct kernel_param *kp)
{
const struct kparam_array *arr = kp->arr;
unsigned int temp_num;
@@ -471,8 +471,9 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
arr->elemsize, arr->ops->set, kp->level,
arr->num ?: &temp_num);
}
+EXPORT_SYMBOL(param_array_set);
-static int param_array_get(char *buffer, const struct kernel_param *kp)
+int param_array_get(char *buffer, const struct kernel_param *kp)
{
int i, off, ret;
const struct kparam_array *arr = kp->arr;
@@ -492,8 +493,9 @@ static int param_array_get(char *buffer, const struct kernel_param *kp)
buffer[off] = '\0';
return off;
}
+EXPORT_SYMBOL(param_array_get);
-static void param_array_free(void *arg)
+void param_array_free(void *arg)
{
unsigned int i;
const struct kparam_array *arr = arg;
@@ -502,6 +504,7 @@ static void param_array_free(void *arg)
for (i = 0; i < (arr->num ? *arr->num : arr->max); i++)
arr->ops->free(arr->elem + arr->elemsize * i);
}
+EXPORT_SYMBOL(param_array_free);
const struct kernel_param_ops param_array_ops = {
.set = param_array_set,
--
2.51.0
^ permalink raw reply related
* [PATCH v2 1/1] module: replace use of system_wq with system_dfl_wq
From: Marco Crivellari @ 2025-09-18 8:55 UTC (permalink / raw)
To: linux-kernel, linux-modules
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Luis Chamberlain, Petr Pavlu
In-Reply-To: <20250918085525.122429-1-marco.crivellari@suse.com>
Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.
This specific patch replace system_wq with system_dfl_wq, the new unbound
workqueue, because the users does not benefit from a per-cpu wq.
Suggested-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
kernel/module/dups.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/module/dups.c b/kernel/module/dups.c
index bd2149fbe117..0b633f2edda6 100644
--- a/kernel/module/dups.c
+++ b/kernel/module/dups.c
@@ -113,7 +113,7 @@ static void kmod_dup_request_complete(struct work_struct *work)
* let this linger forever as this is just a boot optimization for
* possible abuses of vmalloc() incurred by finit_module() thrashing.
*/
- queue_delayed_work(system_wq, &kmod_req->delete_work, 60 * HZ);
+ queue_delayed_work(system_dfl_wq, &kmod_req->delete_work, 60 * HZ);
}
bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret)
@@ -240,7 +240,7 @@ void kmod_dup_request_announce(char *module_name, int ret)
* There is no rush. But we also don't want to hold the
* caller up forever or introduce any boot delays.
*/
- queue_work(system_wq, &kmod_req->complete_work);
+ queue_work(system_dfl_wq, &kmod_req->complete_work);
out:
mutex_unlock(&kmod_dup_mutex);
--
2.51.0
^ permalink raw reply related
* [PATCH v2 0/1] module: replace wq users and add WQ_PERCPU to alloc_workqueue() users
From: Marco Crivellari @ 2025-09-18 8:55 UTC (permalink / raw)
To: linux-kernel, linux-modules
Cc: Tejun Heo, Lai Jiangshan, Frederic Weisbecker,
Sebastian Andrzej Siewior, Marco Crivellari, Michal Hocko,
Luis Chamberlain, Petr Pavlu
Hi!
Below is a summary of a discussion about the Workqueue API and cpu isolation
considerations. Details and more information are available here:
"workqueue: Always use wq_select_unbound_cpu() for WORK_CPU_UNBOUND."
https://lore.kernel.org/all/20250221112003.1dSuoGyc@linutronix.de/
=== Current situation: problems ===
Let's consider a nohz_full system with isolated CPUs: wq_unbound_cpumask is
set to the housekeeping CPUs, for !WQ_UNBOUND the local CPU is selected.
This leads to different scenarios if a work item is scheduled on an isolated
CPU where "delay" value is 0 or greater then 0:
schedule_delayed_work(, 0);
This will be handled by __queue_work() that will queue the work item on the
current local (isolated) CPU, while:
schedule_delayed_work(, 1);
Will move the timer on an housekeeping CPU, and schedule the work there.
Currently if a user enqueue a work item using schedule_delayed_work() the
used wq is "system_wq" (per-cpu wq) while queue_delayed_work() use
WORK_CPU_UNBOUND (used when a cpu is not specified). The same applies to
schedule_work() that is using system_wq and queue_work(), that makes use
again of WORK_CPU_UNBOUND.
This lack of consistentcy cannot be addressed without refactoring the API.
=== Plan and future plans ===
This patchset is the first stone on a refactoring needed in order to
address the points aforementioned; it will have a positive impact also
on the cpu isolation, in the long term, moving away percpu workqueue in
favor to an unbound model.
These are the main steps:
1) API refactoring (that this patch is introducing)
- Make more clear and uniform the system wq names, both per-cpu and
unbound. This to avoid any possible confusion on what should be
used.
- Introduction of WQ_PERCPU: this flag is the complement of WQ_UNBOUND,
introduced in this patchset and used on all the callers that are not
currently using WQ_UNBOUND.
WQ_UNBOUND will be removed in a future release cycle.
Most users don't need to be per-cpu, because they don't have
locality requirements, because of that, a next future step will be
make "unbound" the default behavior.
2) Check who really needs to be per-cpu
- Remove the WQ_PERCPU flag when is not strictly required.
3) Add a new API (prefer local cpu)
- There are users that don't require a local execution, like mentioned
above; despite that, local execution yeld to performance gain.
This new API will prefer the local execution, without requiring it.
=== Introduced Changes by this series ===
1) [P 1] Replace use of system_wq
system_wq is a per-CPU workqueue, but his name is not clear.
Because of that, system_wq has been renamed in system_percpu_wq.
The actual code doesn't benefit from a per-cpu wq, so instead of
the per-cpu wq, system_dfl_wq has been used.
Thanks!
---
Changes in v2:
- system_wq replaced by system_dfl_wq, the new unbound wq
Marco Crivellari (1):
module: replace use of system_wq with system_dfl_wq
kernel/module/dups.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH v8 8/8] kbuild: vmlinux.unstripped should always depend on .vmlinux.export.o
From: Alexey Gladkov @ 2025-09-18 8:05 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov
In-Reply-To: <cover.1758182101.git.legion@kernel.org>
Since .vmlinux.export.c is used to add generated by modpost modaliases
for builtin modules the .vmlinux.export.o is no longer optional and
should always be created. The generation of this file is not dependent
on CONFIG_MODULES.
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
scripts/Makefile.vmlinux | 9 ++-------
scripts/link-vmlinux.sh | 5 +----
2 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 1e5e37aadcd05..7c6ae9886f8f7 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -53,11 +53,6 @@ endif
# vmlinux.unstripped
# ---------------------------------------------------------------------------
-ifdef CONFIG_MODULES
-targets += .vmlinux.export.o
-vmlinux.unstripped: .vmlinux.export.o
-endif
-
ifdef CONFIG_ARCH_WANTS_PRE_LINK_VMLINUX
vmlinux.unstripped: arch/$(SRCARCH)/tools/vmlinux.arch.o
@@ -72,8 +67,8 @@ cmd_link_vmlinux = \
$< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)" "$@"; \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
-targets += vmlinux.unstripped
-vmlinux.unstripped: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
+targets += vmlinux.unstripped .vmlinux.export.o
+vmlinux.unstripped: scripts/link-vmlinux.sh vmlinux.o .vmlinux.export.o $(KBUILD_LDS) FORCE
+$(call if_changed_dep,link_vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF
vmlinux.unstripped: $(RESOLVE_BTFIDS)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 51367c2bfc21e..433849ff7529e 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -73,10 +73,7 @@ vmlinux_link()
objs="${objs} .builtin-dtbs.o"
fi
- if is_enabled CONFIG_MODULES; then
- objs="${objs} .vmlinux.export.o"
- fi
-
+ objs="${objs} .vmlinux.export.o"
objs="${objs} init/version-timestamp.o"
if [ "${SRCARCH}" = "um" ]; then
--
2.51.0
^ permalink raw reply related
* [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Alexey Gladkov @ 2025-09-18 8:05 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez
Cc: linux-kernel, linux-modules, linux-kbuild, Alexey Gladkov,
Masahiro Yamada, Stephen Rothwell
In-Reply-To: <cover.1758182101.git.legion@kernel.org>
For some modules, modalias is generated using the modpost utility and
the section is added to the module file.
When a module is added inside vmlinux, modpost does not generate
modalias for such modules and the information is lost.
As a result kmod (which uses modules.builtin.modinfo in userspace)
cannot determine that modalias is handled by a builtin kernel module.
$ cat /sys/devices/pci0000:00/0000:00:14.0/modalias
pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
$ modinfo xhci_pci
name: xhci_pci
filename: (builtin)
license: GPL
file: drivers/usb/host/xhci-pci
description: xHCI PCI Host Controller Driver
Missing modalias "pci:v*d*sv*sd*bc0Csc03i30*" which will be generated by
modpost if the module is built separately.
To fix this it is necessary to generate the same modalias for vmlinux as
for the individual modules. Fortunately '.vmlinux.export.o' is already
generated from which '.modinfo' can be extracted in the same way as for
vmlinux.o.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
include/linux/module.h | 4 ----
scripts/Makefile.vmlinux | 4 +++-
scripts/mksysmap | 3 +++
scripts/mod/file2alias.c | 19 ++++++++++++++++++-
scripts/mod/modpost.c | 15 +++++++++++++++
scripts/mod/modpost.h | 2 ++
6 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e31ee29fac6b7..e135cc79aceea 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -256,14 +256,10 @@ struct module_kobject *lookup_or_create_module_kobject(const char *name);
__PASTE(type, \
__PASTE(__, name)))))
-#ifdef MODULE
/* Creates an alias so file2alias.c can find device table. */
#define MODULE_DEVICE_TABLE(type, name) \
static typeof(name) __mod_device_table(type, name) \
__attribute__ ((used, alias(__stringify(name))))
-#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
-#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
* Or for CVS/RCS ID version, everything but the number is stripped.
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index ce79461714979..1e5e37aadcd05 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -89,11 +89,13 @@ endif
remove-section-y := .modinfo
remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
+remove-symbols := -w --strip-symbol='__mod_device_table__*'
+
# To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
# it is necessary to remove the PT_LOAD flag from the segment.
quiet_cmd_strip_relocs = OBJCOPY $@
cmd_strip_relocs = $(OBJCOPY) $(patsubst %,--set-section-flags %=noload,$(remove-section-y)) $< $@; \
- $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) $@
+ $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) $(remove-symbols) $@
targets += vmlinux
vmlinux: vmlinux.unstripped FORCE
diff --git a/scripts/mksysmap b/scripts/mksysmap
index a607a0059d119..c4531eacde202 100755
--- a/scripts/mksysmap
+++ b/scripts/mksysmap
@@ -59,6 +59,9 @@
# EXPORT_SYMBOL (namespace)
/ __kstrtabns_/d
+# MODULE_DEVICE_TABLE (symbol name)
+/ __mod_device_table__/d
+
# ---------------------------------------------------------------------------
# Ignored suffixes
# (do not forget '$' after each pattern)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 1260bc2287fba..7da9735e7ab3e 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1477,7 +1477,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
void *symval;
char *zeros = NULL;
const char *type, *name, *modname;
- size_t typelen;
+ size_t typelen, modnamelen;
static const char *prefix = "__mod_device_table__";
/* We're looking for a section relative symbol */
@@ -1500,6 +1500,7 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
type = strstr(modname, "__");
if (!type)
return;
+ modnamelen = type - modname;
type += strlen("__");
name = strstr(type, "__");
@@ -1526,5 +1527,21 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
}
}
+ if (mod->is_vmlinux) {
+ struct module_alias *alias;
+
+ /*
+ * If this is vmlinux, record the name of the builtin module.
+ * Traverse the linked list in the reverse order, and set the
+ * builtin_modname unless it has already been set in the
+ * previous call.
+ */
+ list_for_each_entry_reverse(alias, &mod->aliases, node) {
+ if (alias->builtin_modname)
+ break;
+ alias->builtin_modname = xstrndup(modname, modnamelen);
+ }
+ }
+
free(zeros);
}
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5ca7c268294eb..47c8aa2a69392 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2067,11 +2067,26 @@ static void write_if_changed(struct buffer *b, const char *fname)
static void write_vmlinux_export_c_file(struct module *mod)
{
struct buffer buf = { };
+ struct module_alias *alias, *next;
buf_printf(&buf,
"#include <linux/export-internal.h>\n");
add_exported_symbols(&buf, mod);
+
+ buf_printf(&buf,
+ "#include <linux/module.h>\n"
+ "#undef __MODULE_INFO_PREFIX\n"
+ "#define __MODULE_INFO_PREFIX\n");
+
+ list_for_each_entry_safe(alias, next, &mod->aliases, node) {
+ buf_printf(&buf, "MODULE_INFO(%s.alias, \"%s\");\n",
+ alias->builtin_modname, alias->str);
+ list_del(&alias->node);
+ free(alias->builtin_modname);
+ free(alias);
+ }
+
write_if_changed(&buf, ".vmlinux.export.c");
free(buf.p);
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 9133e4c3803f0..2aecb8f25c87e 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -99,10 +99,12 @@ buf_write(struct buffer *buf, const char *s, int len);
* struct module_alias - auto-generated MODULE_ALIAS()
*
* @node: linked to module::aliases
+ * @modname: name of the builtin module (only for vmlinux)
* @str: a string for MODULE_ALIAS()
*/
struct module_alias {
struct list_head node;
+ char *builtin_modname;
char str[];
};
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox