LinuxLists.cc - [PATCH v2 2/3] memory: export symbols for memory related functions

2023-06-14 03:28:27

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

In this patch, we modified 3 files and export symbols
for 2 functions.
Export symbols for "smap_gather_stats" functions so that
user can have an idea for each user process memory's usage.
Export symbols for "arch_vma_name" functions so that
user can know the heap usage for each user process.
According to these two information, user can do the memory
statistics and anaysis.

Signed-off-by: Wei Chin Tsai <[email protected]>
---
arch/arm/kernel/process.c | 1 +
fs/proc/task_mmu.c | 5 +++--
kernel/signal.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)

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);

/* If possible, provide a placement hint at a random offset from the
* stack for the sigpage and vdso pages.
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6259dd432eeb..814d7829a20b 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -773,8 +773,8 @@ static const struct mm_walk_ops smaps_shmem_walk_ops = {
*
* Use vm_start of @vma as the beginning address if @start is 0.
*/
-static void smap_gather_stats(struct vm_area_struct *vma,
- struct mem_size_stats *mss, unsigned long start)
+void smap_gather_stats(struct vm_area_struct *vma,
+ struct mem_size_stats *mss, unsigned long start)
{
const struct mm_walk_ops *ops = &smaps_walk_ops;

@@ -809,6 +809,7 @@ static void smap_gather_stats(struct vm_area_struct *vma,
else
walk_page_range(vma->vm_mm, start, vma->vm_end, ops, mss);
}
+EXPORT_SYMBOL_GPL(smap_gather_stats);

#define SEQ_PUT_DEC(str, val) \
seq_put_decimal_ull_width(m, str, (val) >> 10, 8)
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);

static inline void siginfo_buildtime_checks(void)
{
--
2.18.0

2023-06-14 07:43:22

by Russell King (Oracle)

[permalink] [raw]

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

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!

2023-06-14 09:18:34

by kernel test robot

[permalink] [raw]

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

Hi Wei,

kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.4-rc6 next-20230613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Wei-Chin-Tsai/kernel-process-fork-exit-export-symbol-for-fork-exit-tracing-functions/20230614-112218
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20230614032038.11699-3-Wei-chin.Tsai%40mediatek.com
patch subject: [PATCH v2 2/3] memory: export symbols for memory related functions
config: hexagon-randconfig-r041-20230612 (https://download.01.org/0day-ci/archive/20230614/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
git fetch char-misc char-misc-testing
git checkout char-misc/char-misc-testing
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash fs/proc/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from fs/proc/task_mmu.c:3:
In file included from include/linux/mm_inline.h:7:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from fs/proc/task_mmu.c:3:
In file included from include/linux/mm_inline.h:7:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from fs/proc/task_mmu.c:3:
In file included from include/linux/mm_inline.h:7:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> fs/proc/task_mmu.c:776:6: warning: no previous prototype for function 'smap_gather_stats' [-Wmissing-prototypes]
776 | void smap_gather_stats(struct vm_area_struct *vma,
| ^
fs/proc/task_mmu.c:776:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
776 | void smap_gather_stats(struct vm_area_struct *vma,
| ^
| static
7 warnings generated.

vim +/smap_gather_stats +776 fs/proc/task_mmu.c

769
770 /*
771 * Gather mem stats from @vma with the indicated beginning
772 * address @start, and keep them in @mss.
773 *
774 * Use vm_start of @vma as the beginning address if @start is 0.
775 */
> 776 void smap_gather_stats(struct vm_area_struct *vma,
777 struct mem_size_stats *mss, unsigned long start)
778 {
779 const struct mm_walk_ops *ops = &smaps_walk_ops;
780
781 /* Invalid start */
782 if (start >= vma->vm_end)
783 return;
784
785 if (vma->vm_file && shmem_mapping(vma->vm_file->f_mapping)) {
786 /*
787 * For shared or readonly shmem mappings we know that all
788 * swapped out pages belong to the shmem object, and we can
789 * obtain the swap value much more efficiently. For private
790 * writable mappings, we might have COW pages that are
791 * not affected by the parent swapped out pages of the shmem
792 * object, so we have to distinguish them during the page walk.
793 * Unless we know that the shmem object (or the part mapped by
794 * our VMA) has no swapped out pages at all.
795 */
796 unsigned long shmem_swapped = shmem_swap_usage(vma);
797
798 if (!start && (!shmem_swapped || (vma->vm_flags & VM_SHARED) ||
799 !(vma->vm_flags & VM_WRITE))) {
800 mss->swap += shmem_swapped;
801 } else {
802 ops = &smaps_shmem_walk_ops;
803 }
804 }
805
806 /* mmap_lock is held in m_start */
807 if (!start)
808 walk_page_vma(vma, ops, mss);
809 else
810 walk_page_range(vma->vm_mm, start, vma->vm_end, ops, mss);
811 }
812 EXPORT_SYMBOL_GPL(smap_gather_stats);
813

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-14 10:23:38

by kernel test robot

[permalink] [raw]

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

2023-06-14 11:36:20

by Wei-chin Tsai (蔡維晉)

[permalink] [raw]

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

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.

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.

Thanks.

Regards,

Jim

2023-06-14 12:26:00

by AngeloGioacchino Del Regno

[permalink] [raw]

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

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.
>
> Thanks.
>
> Regards,
>
> Jim
>

2023-06-14 14:39:13

by kernel test robot

[permalink] [raw]

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

Hi Wei,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.4-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Wei-Chin-Tsai/kernel-process-fork-exit-export-symbol-for-fork-exit-tracing-functions/20230614-112218
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20230614032038.11699-3-Wei-chin.Tsai%40mediatek.com
patch subject: [PATCH v2 2/3] memory: export symbols for memory related functions
config: arm-randconfig-r033-20230612 (https://download.01.org/0day-ci/archive/20230614/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
git fetch char-misc char-misc-testing
git checkout char-misc/char-misc-testing
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: vmlinux: 'arch_vma_name' exported twice. Previous export was in vmlinux
WARNING: modpost: EXPORT symbol "arch_vma_name" [vmlinux] version generation failed, symbol will not be versioned.
Is "arch_vma_name" prototyped in <asm/asm-prototypes.h>?

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2023-06-14 16:20:54

by Wei-chin Tsai (蔡維晉)

[permalink] [raw]

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

On Wed, 2023-06-14 at 14:11 +0200, AngeloGioacchino Del Regno wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 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.
> >
> > Thanks.
> >
> > Regards,
> >
> > Jim
> >
>

Hi Russell and Angelo,

Please use the following patch to see if the problem still exists.
Thanks.

From 150162ce2365557710e9ac8ef62c59f870ffbbb0 Mon Sep 17 00:00:00 2001
From: Wei Chin Tsai <[email protected]>
Date: Wed, 14 Jun 2023 23:31:02 +0800
Subject: [PATCH v3 1/1] memory: Fix export symbol twice compiler error
for
"export symbols for memory related functions" patch

User could not add the export symbol "arch_vma_name"
in arch/arm/kernel/process.c and kernel/signal.c both.
It would cause the export symbol twice compiler error
Reported-by: kernel test robot <[email protected]>

Signed-off-by: Wei Chin Tsai <[email protected]>
---
arch/arm/kernel/process.c | 3 +++
kernel/signal.c | 3 +++
2 files changed, 6 insertions(+)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index df91412a1069..d71a9bafb584 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -343,7 +343,10 @@ const char *arch_vma_name(struct vm_area_struct
*vma)
{
return is_gate_vma(vma) ? "[vectors]" : NULL;
}
+
+#ifdef CONFIG_ARM
EXPORT_SYMBOL_GPL(arch_vma_name);
+#endif

/* If possible, provide a placement hint at a random offset from the
* stack for the sigpage and vdso pages.
diff --git a/kernel/signal.c b/kernel/signal.c
index a1abe77fcdc3..f7d03450781e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4700,7 +4700,10 @@ __weak const char *arch_vma_name(struct
vm_area_struct *vma)
{
return NULL;
}
+
+#ifdef CONFIG_ARM64
EXPORT_SYMBOL_GPL(arch_vma_name);
+#endif

static inline void siginfo_buildtime_checks(void)
{
--
2.18.0

Regards,

Jim
>

2023-06-14 16:21:53

by Wei-chin Tsai (蔡維晉)

[permalink] [raw]

Subject: [PATCH v3 1/1] memory: Fix export symbol twice compiler error for "export symbols for memory related functions" patch

User could not add the export symbol "arch_vma_name"
in arch/arm/kernel/process.c and kernel/signal.c both.
It would cause the export symbol twice compiler error
Reported-by: kernel test robot <[email protected]>

Signed-off-by: Wei Chin Tsai <[email protected]>
---
arch/arm/kernel/process.c | 3 +++
kernel/signal.c | 3 +++
2 files changed, 6 insertions(+)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index df91412a1069..d71a9bafb584 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -343,7 +343,10 @@ const char *arch_vma_name(struct vm_area_struct *vma)
{
return is_gate_vma(vma) ? "[vectors]" : NULL;
}
+
+#ifdef CONFIG_ARM
EXPORT_SYMBOL_GPL(arch_vma_name);
+#endif

/* If possible, provide a placement hint at a random offset from the
* stack for the sigpage and vdso pages.
diff --git a/kernel/signal.c b/kernel/signal.c
index a1abe77fcdc3..f7d03450781e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4700,7 +4700,10 @@ __weak const char *arch_vma_name(struct vm_area_struct *vma)
{
return NULL;
}
+
+#ifdef CONFIG_ARM64
EXPORT_SYMBOL_GPL(arch_vma_name);
+#endif

static inline void siginfo_buildtime_checks(void)
{
--
2.18.0

2023-06-14 16:35:56

by Russell King (Oracle)

[permalink] [raw]

Subject: Re: [PATCH v3 1/1] memory: Fix export symbol twice compiler error for "export symbols for memory related functions" patch

On Wed, Jun 14, 2023 at 11:39:02PM +0800, Wei Chin Tsai wrote:
> User could not add the export symbol "arch_vma_name"
> in arch/arm/kernel/process.c and kernel/signal.c both.
> It would cause the export symbol twice compiler error
> Reported-by: kernel test robot <[email protected]>
>
> Signed-off-by: Wei Chin Tsai <[email protected]>

I'm sorry, but this patch is silly.

> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index df91412a1069..d71a9bafb584 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -343,7 +343,10 @@ const char *arch_vma_name(struct vm_area_struct *vma)
> {
> return is_gate_vma(vma) ? "[vectors]" : NULL;
> }
> +
> +#ifdef CONFIG_ARM
> EXPORT_SYMBOL_GPL(arch_vma_name);
> +#endif

CONFIG_ARM will always be set here, so adding this ifdef is useless.

> diff --git a/kernel/signal.c b/kernel/signal.c
> index a1abe77fcdc3..f7d03450781e 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -4700,7 +4700,10 @@ __weak const char *arch_vma_name(struct vm_area_struct *vma)
> {
> return NULL;
> }
> +
> +#ifdef CONFIG_ARM64
> EXPORT_SYMBOL_GPL(arch_vma_name);
> +#endif

Sorry, but no.

Please do the research I've now twice asked for.

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

2023-06-14 16:36:16

by Russell King (Oracle)

[permalink] [raw]

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

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!

2023-06-14 17:21:32

by Matthew Wilcox

[permalink] [raw]

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

On Wed, Jun 14, 2023 at 05:21:43PM +0100, Russell King (Oracle) wrote:
> 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.

I think the patch should be NAKed indefinitely. I had a quick look at
the user, and it seems like something is being done in the kernel that
should be done in userspace.

2023-06-15 02:19:34

by Wei-chin Tsai (蔡維晉)

[permalink] [raw]

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

On Wed, 2023-06-14 at 17:21 +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 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/

Hi Russell,

OK, I will do the research for your questions...

Thanks.

> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!