Re: [PATCH/RFC] module: replace module_layout with module_memory

Le 09/01/2023 à 21:51, Song Liu a écrit :

On Mon, Jan 9, 2023 at 10:24 AM Song Liu [off-list ref] wrote:

quoted

On Mon, Jan 9, 2023 at 10:03 AM Christophe Leroy
[off-list ref] wrote:

quoted

Le 06/01/2023 à 23:09, Song Liu a écrit :

quoted

module_layout manages different types of memory (text, data, rodata, etc.)
in one allocation, which is problematic for some reasons:

1. It is hard to enable CONFIG_STRICT_MODULE_RWX.
2. It is hard to use huge pages in modules (and not break strict rwx).
3. Many archs uses module_layout for arch-specific data, but it is not
     obvious how these data are used (are they RO, RX, or RW?)

Improve the scenario by replacing 2 (or 3) module_layout per module with
up to 7 module_memory per module:

          MOD_MEM_TYPE_TEXT,
          MOD_MEM_TYPE_DATA,
          MOD_MEM_TYPE_RODATA,
          MOD_MEM_TYPE_RO_AFTER_INIT,
          MOD_MEM_TYPE_INIT_TEXT,
          MOD_MEM_TYPE_INIT_DATA,
          MOD_MEM_TYPE_INIT_RODATA,

and allocating them separately.

Various archs use module_layout for different data. These data are put
into different module_memory based on their location in module_layout.
IOW, data that used to go with text is allocated with MOD_MEM_TYPE_TEXT;
data that used to go with data is allocated with MOD_MEM_TYPE_DATA, etc.

I dislike how it looks with enums, things like
mod->mod_mem[MOD_MEM_TYPE_INIT_TEXT] are odd and don't read nicely.
Could we have something nicer like mod->mod_mem_init_text ?
I know it will complicate your for_each_mod_mem_type() but it would look
nicer.

Hmm.. I am not sure whether we want 7 module_memory here. But if we
agree that it looks better like that, I am ok with it.

quoted

Also, can you explain how you switch from two trees to only one ?
As far as I remember, the same question arised when I implemented
CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC, and the conclusion was that
we had to keep two independant trees, so I'm a bit puzzled that you have
now merged everything into a single tree.

AFAICT, we only need __module_address() to work? So one tree is enough.
Did I miss something?

Do you mean one tree will cause addr_[min|max] to be inaccurate?

Yes at least. On powerpc you will have module text below kernel, 
somewhere between 0xb0000000 and 0xcfffffff, and you will have module 
data in vmalloc area, somewhere between 0xf0000000 and 0xffffffff.

If you have only one tree, any address between 0xc0000000 and 0xefffffff 
will trigger a tree search.

Christophe