Re: [PATCH v2 2/3] memory: export symbols for memory related functions

[Russell King (Oracle)]

On Wed, Jun 14, 2023 at 02:11:25PM +0200, AngeloGioacchino Del Regno wrote:
> Il 14/06/23 11:59, Wei-chin Tsai (蔡維晉) ha scritto:
> > On Wed, 2023-06-14 at 08:16 +0100, Russell King (Oracle) wrote:
> > >   	
> > > External email : Please do not click links or open attachments until
> > > you have verified the sender or the content.
> > >   On Wed, Jun 14, 2023 at 11:20:34AM +0800, Wei Chin Tsai wrote:
> > > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> > > > index 0e8ff85890ad..df91412a1069 100644
> > > > --- a/arch/arm/kernel/process.c
> > > > +++ b/arch/arm/kernel/process.c
> > > > @@ -343,6 +343,7 @@ const char *arch_vma_name(struct vm_area_struct
> > > *vma)
> > > >   {
> > > >   return is_gate_vma(vma) ? "[vectors]" : NULL;
> > > >   }
> > > > +EXPORT_SYMBOL_GPL(arch_vma_name);
> > > ...
> > > > diff --git a/kernel/signal.c b/kernel/signal.c
> > > > index b5370fe5c198..a1abe77fcdc3 100644
> > > > --- a/kernel/signal.c
> > > > +++ b/kernel/signal.c
> > > > @@ -4700,6 +4700,7 @@ __weak const char *arch_vma_name(struct
> > > vm_area_struct *vma)
> > > >   {
> > > >   return NULL;
> > > >   }
> > > > +EXPORT_SYMBOL_GPL(arch_vma_name);
> > > 
> > > Have you confirmed:
> > > 1) whether this actually builds
> > > 2) whether this results in one or two arch_vma_name exports
> > > 
> > > ?
> > > 
> > > -- 
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> > 
> > Hi Russell,
> > 
> > We did confirm that it can be built successfully in kernel 6.1 and run
> > well in our system.
> > 
> 
> It runs well in your system and can be built successfully because you're building
> for ARM64, not for ARM...
> 
> > Actually, we only use this export symbol "arch_vma_name"
> > from kernel/signal.c in arm64. We also export symbol for arch_vma_name
> > in arch/arm/kernel/process.c because that, one day in the future,  we
> > are afraid that we also need this function in arm platform.

What I'm trying to get at is that we have arch_vma_name in
arch/arm/kernel/process.c and also a weak function in kernel/signal.c.

Both of these end up adding an entry into the __ksymtab_strings
section and a ___ksymtab section for this symbol. So we end up with
two entries in each.

Now, if the one from kernel/signal.c points at its own weak function,
and that is found first, then that's the function that is going to be
bound, not the function that's overriding it.

If, instead, the export in kernel/signal.c ends up pointing at the
overriden function, then the export in arch/arm/kernel/process.c is
entirely redundant.

So, you need to get to the bottom of this... and until you do I'm
afraid I'll have to NAK this patch.

For the record, I suspect it's the latter scenario (we end up with
two entries pointing at the same function) but that's nothing more
than a hunch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!