Re: [PATCH 1/1] alloc_tag: remove empty module tag section from linker script

[Suren Baghdasaryan <surenb@google.com>]

On Mon, Jun 9, 2025 at 2:15 PM Casey Chen <cachen@purestorage.com> wrote:
>
> On Thu, Jun 5, 2025 at 1:07 PM Suren Baghdasaryan <surenb@google.com> wrote:
> >
> > On Thu, Jun 5, 2025 at 12:01 PM Casey Chen <cachen@purestorage.com> wrote:
> > >
> > > The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> > > section in module linker script, which caused symbol lookup tools
> > > like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> > > incorrectly mapping to unrelated functions like below.
> > >
> > >   (gdb) disas __ib_process_cq
> > >   Dump of assembler code for function trace_event_fields_cq_schedule:
> > >
> > > Removing the empty section restores proper symbol resolution and
> > > layout, ensuring .data placement behaves as expected.
> >
> > Hmm. I'm not sure why an empty .data section would cause such an
> > issue. Is that expected behavior?
> >
>
> I'm not sure that's why I am posting this to ask for ideas. It looks
> like gdb failed to disassemble function.
>
> > To clarify, codetags are designed to support different types of tags,
> > not only allocation tags which we currently use. It so happens that
> > allocation tags can be still used after module unload, therefore they
> > are placed into MOD_SEPARATE_CODETAG_SECTIONS(). If some other tags
> > are added in the future and their lifecycle is the same as modules
> > (IOW after module unload they can be unloaded too), then they would be
> > added into MOD_CODETAG_SECTIONS() but until then this section is
> > empty.
> >
>
> Could we remove the empty data section with MOD_CODETAG_SECTIONS() for
> now until we really need it ?

Ok, I see no issue with removing it but please remove its definition
from codetag.lds.h as well.

Whenever sending mm related patches (including alloc_tag ones) please
send them to akpm@linux-foundation.org and CC everyone else. Andrew is
the mm tree maintainer and that's the usual way to post MM changes.
Thanks,
Suren.

> > >
> > > Fixes: 22d407b164ff ("lib: add allocation tagging support for memory allocation profiling")
> > > Signed-off-by: Casey Chen <cachen@purestorage.com>
> > > Reviewed-by: Yuanyuan Zhong <yzhong@purestorage.com>
> > > ---
> > >  scripts/module.lds.S | 5 -----
> > >  1 file changed, 5 deletions(-)
> > >
> > > diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> > > index 711c6e029936..c071ca4beedd 100644
> > > --- a/scripts/module.lds.S
> > > +++ b/scripts/module.lds.S
> > > @@ -50,17 +50,12 @@ SECTIONS {
> > >         .data : {
> > >                 *(.data .data.[0-9a-zA-Z_]*)
> > >                 *(.data..L*)
> > > -               MOD_CODETAG_SECTIONS()
> > >         }
> > >
> > >         .rodata : {
> > >                 *(.rodata .rodata.[0-9a-zA-Z_]*)
> > >                 *(.rodata..L*)
> > >         }
> > > -#else
> > > -       .data : {
> > > -               MOD_CODETAG_SECTIONS()
> > > -       }
> > >  #endif
> > >         MOD_SEPARATE_CODETAG_SECTIONS()
> > >  }
> > > --
> > > 2.34.1
> > >