> On Feb 8, 2023, at 10:37 PM, Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
>
>
>
> Le 09/02/2023 à 01:16, Song Liu a écrit :
>>
>>
>>> On Feb 8, 2023, at 9:48 AM, Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
>>
>> [...]
>>
>>>> {
>>>> unsigned long min = (unsigned long)base;
>>>> unsigned long max = min + size;
>>>>
>>>> +#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
>>>
>>> A #ifdef shouldn't be required. You can use IS_ENABLED() instead:
>>>
>>>
>>>
>>>> + if (mod_mem_type_is_core_data(type)) {
>>>
>>> if (IS_ENABLED(CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC) &&
>>> mod_mem_type_is_core_data(type))
>>>
>>>> + if (min < tree->data_addr_min)
>>>> + tree->data_addr_min = min;
>>>> + if (max > tree->data_addr_max)
>>>> + tree->data_addr_max = max;
>>>> + return;
>>>> + }
>>>> +#endif
>>
>> To use IS_ENABLED() here, we also need to keep data_addr_[min|max]
>> around. Do we really want them?
>
> It is up to you. If you think it is not worth the effort, it's fine for me.
>
> Allthough it could probably be easily fixed by doing (untested) :
>
> struct mod_tree_root {
> #ifdef CONFIG_MODULES_TREE_LOOKUP
> struct latch_tree_root root;
> #endif
> #ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> unsigned long addr_min;
> unsigned long addr_max;
> unsigned long data_addr_min;
> unsigned long data_addr_max;
> #else
> union {
> unsigned long addr_min;
> unsigned long data_addr_min;
> };
> union {
> unsigned long addr_max;
> unsigned long data_addr_max;
> }:
> #endif
> };
I think using union here will be an overkill. Let's just keep it
simple with #ifdef.
Thanks,
Song
