Re: [PATCH v1 5/7] modpost: Create modalias for builtin modules

[Petr Pavlu <petr.pavlu@suse.com>]

On 4/29/25 17:15, Alexey Gladkov wrote:
> On Tue, Apr 29, 2025 at 04:14:13PM +0200, Petr Pavlu wrote:
>> On 4/29/25 14:49, Alexey Gladkov wrote:
>>> On Tue, Apr 29, 2025 at 12:04:44PM +0200, Alexey Gladkov wrote:
>>>>> I'm not sure it's best to overload this data in this way. I think mixing
>>>>> actual files and "logical" modules in the modules list is somewhat
>>>>> confusing.
>>>>>
>>>>> An alternative would be to keep a single module struct for vmlinux and
>>>>> record the discovered aliases under it?
>>>>
>>>> It is possible to extend struct module_alias and add the module name. The
>>>> problem is that alias is added by module_alias_printf() and we will have
>>>> to add the module name to the arguments to each do_entry handler in
>>>> addition to struct module where there is already a name (but in our case
>>>> it is vmlinux).
>>>>
>>>> I can do that if you think it's a better way.
>>>
>>> If I don't add separate entries for each builtin module, the patch will
>>> look like this:
>>> [...]
>>
>> I see, that didn't turn out as well as I envisioned. One more approach
>> would be to track builtin modules separately. A patch is below. I'm not
>> sure if it's better.
> 
> I'm not sure I get it. What do you mean when you say I need to track
> builtin modules separately ?

The patch that I sent in my reply introduces a new list called
builtin_modules. This is what I meant by tracking builtin modules
separately. This implementation has the advantage of not conceptually
mixing the modules and allows the function write_vmlinux_export_c_file()
to directly use the new list.

On the other hand, keeping everything in one list and introducing a flag
for builtin modules (or maybe replacing module.is_vmlinux with some
module.type enum) allows functions that take only a module pointer to
know which module they are dealing with, which I imagine could be
useful.

I don't have a clear preference, as long as we avoid misusing
module.dump_file.

-- 
Thanks,
Petr