Re: linux-next failing build due to missing cubictcp_state symbol

Re: linux-next failing build due to missing cubictcp_state symbol

[Jiri Slaby]

[-- Attachment #1: Type: text/plain, Size: 2383 bytes --]

CCing pahole people.

On 03. 05. 21, 9:59, Jiri Slaby wrote:
> On 03. 05. 21, 8:11, Jiri Slaby wrote:
>>>>>>> looks like vfs_truncate did not get into BTF data,
>>>>>>> I'll try to reproduce
>>>
>>> _None_ of the functions are generated by pahole -J from debuginfo on 
>>> ppc64. debuginfo appears to be correct. Neither pahole -J fs/open.o 
>>> works correctly. collect_functions in dwarves seems to be defunct on 
>>> ppc64... "functions" array is bogus (so find_function -- the bsearch 
>>> -- fails).
>>
>> It's not that bogus. I forgot an asterisk:
>>> #0  find_function (btfe=0x100269f80, name=0x10024631c "stream_open") 
>>> at /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
>>> (gdb) p (*functions)@84
>>> $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr = 
>>> 75232, size = 72, sh_addr = 65536, generated = false}, {
>>>     name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592, size 
>>> = 216, sh_addr = 65536, generated = false}, {
>>>     name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr = 80816, 
>>> size = 232, sh_addr = 65536, generated = false}, {
>>>     name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr = 74304, 
>>> size = 100, sh_addr = 65536, generated = false}, {
>> ...
>>>     name = 0x7ffff68e0808 ".stream_open", addr = 65824, size = 72, 
>>> sh_addr = 65536, generated = false}, {
>> ...
>>>     name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size = 544, 
>>> sh_addr = 65536, generated = false}}
>>
>> The dot makes the difference, of course. The question is why is it 
>> there? I keep looking into it. Only if someone has an immediate idea...
> 
> Well, .vfs_truncate is in .text (and contains an ._mcount call). And 
> vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions 
> excludes all functions without the ._mcount call, is_ftrace_func later 
> returns false for such functions and they are filtered before the BTF 
> processing.
> 
> Technically, get_vmlinux_addrs looks at a list of functions between 
> __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> 
> I don't know what the correct fix is (exclude .opd functions from the 
> filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids 
> this too.

Attaching a patch for pahole which fixes the issue, but I have no idea 
whether it is the right fix at all.

> regards,-- 
js
suse labs

[-- Attachment #2: ppc64-opd-fix.patch --]
[-- Type: text/x-patch, Size: 1686 bytes --]

From: Jiri Slaby <jslaby@suse.cz>
Subject: ppc64: .opd section fix
Patch-mainline: submitted 2021/05/03

Functions in the .opd section should be considered valid too. Otherwise,
pahole cannot produce a .BTF section from vmlinux and kernel build
fails on ppc64.
---
 btf_encoder.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -31,6 +31,8 @@ struct funcs_layout {
 	unsigned long mcount_start;
 	unsigned long mcount_stop;
 	unsigned long mcount_sec_idx;
+	unsigned long opd_start;
+	unsigned long opd_stop;
 };
 
 struct elf_function {
@@ -271,11 +273,24 @@ static int is_ftrace_func(struct elf_fun
 	return start <= addrs[r] && addrs[r] < end;
 }
 
+static int is_opd_func(struct elf_function *func, struct funcs_layout *fl)
+{
+	return fl->opd_start <= func->addr && func->addr < fl->opd_stop;
+}
+
 static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl)
 {
 	__u64 *addrs, count, i;
 	int functions_valid = 0;
 	bool kmod = false;
+	GElf_Shdr shdr;
+	Elf_Scn *sec;
+
+	sec = elf_section_by_name(btfe->elf, &btfe->ehdr, &shdr, ".opd", NULL);
+	if (sec) {
+		fl->opd_start = shdr.sh_addr;
+		fl->opd_stop = shdr.sh_addr + shdr.sh_size;
+	}
 
 	/*
 	 * Check if we are processing vmlinux image and
@@ -322,7 +337,8 @@ static int setup_functions(struct btf_el
 			func->addr += func->sh_addr;
 
 		/* Make sure function is within ftrace addresses. */
-		if (is_ftrace_func(func, addrs, count)) {
+		if (is_opd_func(func, fl) ||
+				is_ftrace_func(func, addrs, count)) {
 			/*
 			 * We iterate over sorted array, so we can easily skip
 			 * not valid item and move following valid field into

Re: linux-next failing build due to missing cubictcp_state symbol

[Jiri Olsa]

On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
> CCing pahole people.
> 
> On 03. 05. 21, 9:59, Jiri Slaby wrote:
> > On 03. 05. 21, 8:11, Jiri Slaby wrote:
> > > > > > > > looks like vfs_truncate did not get into BTF data,
> > > > > > > > I'll try to reproduce
> > > > 
> > > > _None_ of the functions are generated by pahole -J from
> > > > debuginfo on ppc64. debuginfo appears to be correct. Neither
> > > > pahole -J fs/open.o works correctly. collect_functions in
> > > > dwarves seems to be defunct on ppc64... "functions" array is
> > > > bogus (so find_function -- the bsearch -- fails).
> > > 
> > > It's not that bogus. I forgot an asterisk:
> > > > #0  find_function (btfe=0x100269f80, name=0x10024631c
> > > > "stream_open") at
> > > > /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
> > > > (gdb) p (*functions)@84
> > > > $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
> > > > = 75232, size = 72, sh_addr = 65536, generated = false}, {
> > > >     name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
> > > > size = 216, sh_addr = 65536, generated = false}, {
> > > >     name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
> > > > 80816, size = 232, sh_addr = 65536, generated = false}, {
> > > >     name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
> > > > 74304, size = 100, sh_addr = 65536, generated = false}, {
> > > ...
> > > >     name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
> > > > 72, sh_addr = 65536, generated = false}, {
> > > ...
> > > >     name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
> > > > 544, sh_addr = 65536, generated = false}}
> > > 
> > > The dot makes the difference, of course. The question is why is it
> > > there? I keep looking into it. Only if someone has an immediate
> > > idea...
> > 
> > Well, .vfs_truncate is in .text (and contains an ._mcount call). And
> > vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
> > excludes all functions without the ._mcount call, is_ftrace_func later
> > returns false for such functions and they are filtered before the BTF
> > processing.
> > 
> > Technically, get_vmlinux_addrs looks at a list of functions between
> > __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> > 
> > I don't know what the correct fix is (exclude .opd functions from the
> > filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
> > this too.
> 
> Attaching a patch for pahole which fixes the issue, but I have no idea
> whether it is the right fix at all.

hi,
we're considering to disable ftrace filter completely,
I guess that would solve this issue for ppc as well

  https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/

jirka

> 
> > regards,--
> js
> suse labs

> From: Jiri Slaby <jslaby@suse.cz>
> Subject: ppc64: .opd section fix
> Patch-mainline: submitted 2021/05/03
> 
> Functions in the .opd section should be considered valid too. Otherwise,
> pahole cannot produce a .BTF section from vmlinux and kernel build
> fails on ppc64.
> ---
>  btf_encoder.c |   18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -31,6 +31,8 @@ struct funcs_layout {
>  	unsigned long mcount_start;
>  	unsigned long mcount_stop;
>  	unsigned long mcount_sec_idx;
> +	unsigned long opd_start;
> +	unsigned long opd_stop;
>  };
>  
>  struct elf_function {
> @@ -271,11 +273,24 @@ static int is_ftrace_func(struct elf_fun
>  	return start <= addrs[r] && addrs[r] < end;
>  }
>  
> +static int is_opd_func(struct elf_function *func, struct funcs_layout *fl)
> +{
> +	return fl->opd_start <= func->addr && func->addr < fl->opd_stop;
> +}
> +
>  static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl)
>  {
>  	__u64 *addrs, count, i;
>  	int functions_valid = 0;
>  	bool kmod = false;
> +	GElf_Shdr shdr;
> +	Elf_Scn *sec;
> +
> +	sec = elf_section_by_name(btfe->elf, &btfe->ehdr, &shdr, ".opd", NULL);
> +	if (sec) {
> +		fl->opd_start = shdr.sh_addr;
> +		fl->opd_stop = shdr.sh_addr + shdr.sh_size;
> +	}
>  
>  	/*
>  	 * Check if we are processing vmlinux image and
> @@ -322,7 +337,8 @@ static int setup_functions(struct btf_el
>  			func->addr += func->sh_addr;
>  
>  		/* Make sure function is within ftrace addresses. */
> -		if (is_ftrace_func(func, addrs, count)) {
> +		if (is_opd_func(func, fl) ||
> +				is_ftrace_func(func, addrs, count)) {
>  			/*
>  			 * We iterate over sorted array, so we can easily skip
>  			 * not valid item and move following valid field into

Re: linux-next failing build due to missing cubictcp_state symbol

[Michal Suchánek]

On Mon, May 03, 2021 at 12:08:02PM +0200, Jiri Olsa wrote:
> On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
> > CCing pahole people.
> > 
> > On 03. 05. 21, 9:59, Jiri Slaby wrote:
> > > On 03. 05. 21, 8:11, Jiri Slaby wrote:
> > > > > > > > > looks like vfs_truncate did not get into BTF data,
> > > > > > > > > I'll try to reproduce
> > > > > 
> > > > > _None_ of the functions are generated by pahole -J from
> > > > > debuginfo on ppc64. debuginfo appears to be correct. Neither
> > > > > pahole -J fs/open.o works correctly. collect_functions in
> > > > > dwarves seems to be defunct on ppc64... "functions" array is
> > > > > bogus (so find_function -- the bsearch -- fails).
> > > > 
> > > > It's not that bogus. I forgot an asterisk:
> > > > > #0  find_function (btfe=0x100269f80, name=0x10024631c
> > > > > "stream_open") at
> > > > > /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
> > > > > (gdb) p (*functions)@84
> > > > > $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
> > > > > = 75232, size = 72, sh_addr = 65536, generated = false}, {
> > > > >     name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
> > > > > size = 216, sh_addr = 65536, generated = false}, {
> > > > >     name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
> > > > > 80816, size = 232, sh_addr = 65536, generated = false}, {
> > > > >     name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
> > > > > 74304, size = 100, sh_addr = 65536, generated = false}, {
> > > > ...
> > > > >     name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
> > > > > 72, sh_addr = 65536, generated = false}, {
> > > > ...
> > > > >     name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
> > > > > 544, sh_addr = 65536, generated = false}}
> > > > 
> > > > The dot makes the difference, of course. The question is why is it
> > > > there? I keep looking into it. Only if someone has an immediate
> > > > idea...
> > > 
> > > Well, .vfs_truncate is in .text (and contains an ._mcount call). And
> > > vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
> > > excludes all functions without the ._mcount call, is_ftrace_func later
> > > returns false for such functions and they are filtered before the BTF
> > > processing.
> > > 
> > > Technically, get_vmlinux_addrs looks at a list of functions between
> > > __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> > > 
> > > I don't know what the correct fix is (exclude .opd functions from the
> > > filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
> > > this too.
> > 
> > Attaching a patch for pahole which fixes the issue, but I have no idea
> > whether it is the right fix at all.
> 
> hi,
> we're considering to disable ftrace filter completely,
> I guess that would solve this issue for ppc as well
> 
>   https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/
> 
Just disabling the ftrace filter in pahole does not seem to fix it.

Is there some other place where it should be disabled?

Thanks

Michal

Re: linux-next failing build due to missing cubictcp_state symbol

[Michal Suchánek]

On Mon, May 03, 2021 at 06:46:56PM +0200, Michal Suchánek wrote:
> On Mon, May 03, 2021 at 12:08:02PM +0200, Jiri Olsa wrote:
> > On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
> > > CCing pahole people.
> > > 
> > > On 03. 05. 21, 9:59, Jiri Slaby wrote:
> > > > On 03. 05. 21, 8:11, Jiri Slaby wrote:
> > > > > > > > > > looks like vfs_truncate did not get into BTF data,
> > > > > > > > > > I'll try to reproduce
> > > > > > 
> > > > > > _None_ of the functions are generated by pahole -J from
> > > > > > debuginfo on ppc64. debuginfo appears to be correct. Neither
> > > > > > pahole -J fs/open.o works correctly. collect_functions in
> > > > > > dwarves seems to be defunct on ppc64... "functions" array is
> > > > > > bogus (so find_function -- the bsearch -- fails).
> > > > > 
> > > > > It's not that bogus. I forgot an asterisk:
> > > > > > #0  find_function (btfe=0x100269f80, name=0x10024631c
> > > > > > "stream_open") at
> > > > > > /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
> > > > > > (gdb) p (*functions)@84
> > > > > > $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
> > > > > > = 75232, size = 72, sh_addr = 65536, generated = false}, {
> > > > > >     name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
> > > > > > size = 216, sh_addr = 65536, generated = false}, {
> > > > > >     name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
> > > > > > 80816, size = 232, sh_addr = 65536, generated = false}, {
> > > > > >     name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
> > > > > > 74304, size = 100, sh_addr = 65536, generated = false}, {
> > > > > ...
> > > > > >     name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
> > > > > > 72, sh_addr = 65536, generated = false}, {
> > > > > ...
> > > > > >     name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
> > > > > > 544, sh_addr = 65536, generated = false}}
> > > > > 
> > > > > The dot makes the difference, of course. The question is why is it
> > > > > there? I keep looking into it. Only if someone has an immediate
> > > > > idea...
> > > > 
> > > > Well, .vfs_truncate is in .text (and contains an ._mcount call). And
> > > > vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
> > > > excludes all functions without the ._mcount call, is_ftrace_func later
> > > > returns false for such functions and they are filtered before the BTF
> > > > processing.
> > > > 
> > > > Technically, get_vmlinux_addrs looks at a list of functions between
> > > > __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> > > > 
> > > > I don't know what the correct fix is (exclude .opd functions from the
> > > > filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
> > > > this too.
> > > 
> > > Attaching a patch for pahole which fixes the issue, but I have no idea
> > > whether it is the right fix at all.
> > 
> > hi,
> > we're considering to disable ftrace filter completely,
> > I guess that would solve this issue for ppc as well
> > 
> >   https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/
> > 
> Just disabling the ftrace filter in pahole does not seem to fix it.
> 
> Is there some other place where it should be disabled?

Nevermind, purging the system dwarves resolved the problem. Although
kbuild detects pahole as /usr/local/bin/pahole the system binaries or
libraries are still used for something.

Thanks

Michal

Re: linux-next failing build due to missing cubictcp_state symbol

[Jiri Slaby]

[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]

On 03. 05. 21, 12:08, Jiri Olsa wrote:
> On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
>> CCing pahole people.
>>
>> On 03. 05. 21, 9:59, Jiri Slaby wrote:
>>> On 03. 05. 21, 8:11, Jiri Slaby wrote:
>>>>>>>>> looks like vfs_truncate did not get into BTF data,
>>>>>>>>> I'll try to reproduce
>>>>>
>>>>> _None_ of the functions are generated by pahole -J from
>>>>> debuginfo on ppc64. debuginfo appears to be correct. Neither
>>>>> pahole -J fs/open.o works correctly. collect_functions in
>>>>> dwarves seems to be defunct on ppc64... "functions" array is
>>>>> bogus (so find_function -- the bsearch -- fails).
>>>>
>>>> It's not that bogus. I forgot an asterisk:
>>>>> #0  find_function (btfe=0x100269f80, name=0x10024631c
>>>>> "stream_open") at
>>>>> /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
>>>>> (gdb) p (*functions)@84
>>>>> $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
>>>>> = 75232, size = 72, sh_addr = 65536, generated = false}, {
>>>>>      name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
>>>>> size = 216, sh_addr = 65536, generated = false}, {
>>>>>      name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
>>>>> 80816, size = 232, sh_addr = 65536, generated = false}, {
>>>>>      name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
>>>>> 74304, size = 100, sh_addr = 65536, generated = false}, {
>>>> ...
>>>>>      name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
>>>>> 72, sh_addr = 65536, generated = false}, {
>>>> ...
>>>>>      name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
>>>>> 544, sh_addr = 65536, generated = false}}
>>>>
>>>> The dot makes the difference, of course. The question is why is it
>>>> there? I keep looking into it. Only if someone has an immediate
>>>> idea...
>>>
>>> Well, .vfs_truncate is in .text (and contains an ._mcount call). And
>>> vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
>>> excludes all functions without the ._mcount call, is_ftrace_func later
>>> returns false for such functions and they are filtered before the BTF
>>> processing.
>>>
>>> Technically, get_vmlinux_addrs looks at a list of functions between
>>> __start_mcount_loc and __stop_mcount_loc and considers only the listed.
>>>
>>> I don't know what the correct fix is (exclude .opd functions from the
>>> filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
>>> this too.
>>
>> Attaching a patch for pahole which fixes the issue, but I have no idea
>> whether it is the right fix at all.
> 
> hi,
> we're considering to disable ftrace filter completely,
> I guess that would solve this issue for ppc as well
> 
>    https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/

Right, the attached patch fixes it for me too.

-- 
js

[-- Attachment #2: 0001-remove-ftrace-filter.patch --]
[-- Type: text/x-patch, Size: 9077 bytes --]

From b0128f1ec6a234c5b40c40d73d55708d34f1a428 Mon Sep 17 00:00:00 2001
From: Jiri Slaby <jslaby@suse.cz>
Date: Tue, 4 May 2021 08:09:50 +0200
Subject: [PATCH] remove ftrace filter

Functions in the .opd section on ppc64 are currently ignored as they
don't contain mcount calls -- they are excluded by the ftrace filter.
Therefore, pahole cannot produce a .BTF section from vmlinux and kernel
build fails on ppc64.

Remove the ftrace filter completely as was discussed:
 https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/
 https://lore.kernel.org/bpf/YI%2FLgjLxo9VCN%2Fd+@krava/
---
 btf_encoder.c | 272 +-------------------------------------------------
 1 file changed, 5 insertions(+), 267 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 80e896961d4e..ba83bb088efa 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -27,17 +27,8 @@
  */
 #define KSYM_NAME_LEN 128
 
-struct funcs_layout {
-	unsigned long mcount_start;
-	unsigned long mcount_stop;
-	unsigned long mcount_sec_idx;
-};
-
 struct elf_function {
 	const char	*name;
-	unsigned long	 addr;
-	unsigned long	 size;
-	unsigned long	 sh_addr;
 	bool		 generated;
 };
 
@@ -98,250 +89,11 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym,
 	}
 
 	functions[functions_cnt].name = name;
-	functions[functions_cnt].addr = elf_sym__value(sym);
-	functions[functions_cnt].size = elf_sym__size(sym);
-	functions[functions_cnt].sh_addr = sh.sh_addr;
 	functions[functions_cnt].generated = false;
 	functions_cnt++;
 	return 0;
 }
 
-static int addrs_cmp(const void *_a, const void *_b)
-{
-	const __u64 *a = _a;
-	const __u64 *b = _b;
-
-	if (*a == *b)
-		return 0;
-	return *a < *b ? -1 : 1;
-}
-
-static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl,
-			     __u64 **paddrs, __u64 *pcount)
-{
-	__u64 *addrs, count, offset;
-	unsigned int addr_size, i;
-	Elf_Data *data;
-	GElf_Shdr shdr;
-	Elf_Scn *sec;
-
-	/* Initialize for the sake of all error paths below. */
-	*paddrs = NULL;
-	*pcount = 0;
-
-	if (!fl->mcount_start || !fl->mcount_stop)
-		return 0;
-
-	/*
-	 * Find mcount addressed marked by __start_mcount_loc
-	 * and __stop_mcount_loc symbols and load them into
-	 * sorted array.
-	 */
-	sec = elf_getscn(btfe->elf, fl->mcount_sec_idx);
-	if (!sec || !gelf_getshdr(sec, &shdr)) {
-		fprintf(stderr, "Failed to get section(%lu) header.\n",
-			fl->mcount_sec_idx);
-		return -1;
-	}
-
-	/* Get address size from processed file's ELF class. */
-	addr_size = gelf_getclass(btfe->elf) == ELFCLASS32 ? 4 : 8;
-
-	offset = fl->mcount_start - shdr.sh_addr;
-	count  = (fl->mcount_stop - fl->mcount_start) / addr_size;
-
-	data = elf_getdata(sec, 0);
-	if (!data) {
-		fprintf(stderr, "Failed to get section(%lu) data.\n",
-			fl->mcount_sec_idx);
-		return -1;
-	}
-
-	addrs = malloc(count * sizeof(addrs[0]));
-	if (!addrs) {
-		fprintf(stderr, "Failed to allocate memory for ftrace addresses.\n");
-		return -1;
-	}
-
-	if (addr_size == sizeof(__u64)) {
-		memcpy(addrs, data->d_buf + offset, count * addr_size);
-	} else {
-		for (i = 0; i < count; i++)
-			addrs[i] = (__u64) *((__u32 *) (data->d_buf + offset + i * addr_size));
-	}
-
-	*paddrs = addrs;
-	*pcount = count;
-	return 0;
-}
-
-static int
-get_kmod_addrs(struct btf_elf *btfe, __u64 **paddrs, __u64 *pcount)
-{
-	__u64 *addrs, count;
-	unsigned int addr_size, i;
-	GElf_Shdr shdr_mcount;
-	Elf_Data *data;
-	Elf_Scn *sec;
-
-	/* Initialize for the sake of all error paths below. */
-	*paddrs = NULL;
-	*pcount = 0;
-
-	/* get __mcount_loc */
-	sec = elf_section_by_name(btfe->elf, &btfe->ehdr, &shdr_mcount,
-				  "__mcount_loc", NULL);
-	if (!sec) {
-		if (btf_elf__verbose) {
-			printf("%s: '%s' doesn't have __mcount_loc section\n", __func__,
-			       btfe->filename);
-		}
-		return 0;
-	}
-
-	data = elf_getdata(sec, NULL);
-	if (!data) {
-		fprintf(stderr, "Failed to data for __mcount_loc section.\n");
-		return -1;
-	}
-
-	/* Get address size from processed file's ELF class. */
-	addr_size = gelf_getclass(btfe->elf) == ELFCLASS32 ? 4 : 8;
-
-	count = data->d_size / addr_size;
-
-	addrs = malloc(count * sizeof(addrs[0]));
-	if (!addrs) {
-		fprintf(stderr, "Failed to allocate memory for ftrace addresses.\n");
-		return -1;
-	}
-
-	if (addr_size == sizeof(__u64)) {
-		memcpy(addrs, data->d_buf, count * addr_size);
-	} else {
-		for (i = 0; i < count; i++)
-			addrs[i] = (__u64) *((__u32 *) (data->d_buf + i * addr_size));
-	}
-
-	/*
-	 * We get Elf object from dwfl_module_getelf function,
-	 * which performs all possible relocations, including
-	 * __mcount_loc section.
-	 *
-	 * So addrs array now contains relocated values, which
-	 * we need take into account when we compare them to
-	 * functions values, see comment in setup_functions
-	 * function.
-	 */
-	*paddrs = addrs;
-	*pcount = count;
-	return 0;
-}
-
-static int is_ftrace_func(struct elf_function *func, __u64 *addrs, __u64 count)
-{
-	__u64 start = func->addr;
-	__u64 addr, end = func->addr + func->size;
-
-	/*
-	 * The invariant here is addr[r] that is the smallest address
-	 * that is >= than function start addr. Except the corner case
-	 * where there is no such r, but for that we have a final check
-	 * in the return.
-	 */
-	size_t l = 0, r = count - 1, m;
-
-	/* make sure we don't use invalid r */
-	if (count == 0)
-		return false;
-
-	while (l < r) {
-		m = l + (r - l) / 2;
-		addr = addrs[m];
-
-		if (addr >= start) {
-			/* we satisfy invariant, so tighten r */
-			r = m;
-		} else {
-			/* m is not good enough as l, maybe m + 1 will be */
-			l = m + 1;
-		}
-	}
-
-	return start <= addrs[r] && addrs[r] < end;
-}
-
-static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl)
-{
-	__u64 *addrs, count, i;
-	int functions_valid = 0;
-	bool kmod = false;
-
-	/*
-	 * Check if we are processing vmlinux image and
-	 * get mcount data if it's detected.
-	 */
-	if (get_vmlinux_addrs(btfe, fl, &addrs, &count))
-		return -1;
-
-	/*
-	 * Check if we are processing kernel module and
-	 * get mcount data if it's detected.
-	 */
-	if (!addrs) {
-		if (get_kmod_addrs(btfe, &addrs, &count))
-			return -1;
-		kmod = true;
-	}
-
-	if (!addrs) {
-		if (btf_elf__verbose)
-			printf("ftrace symbols not detected, falling back to DWARF data\n");
-		delete_functions();
-		return 0;
-	}
-
-	qsort(addrs, count, sizeof(addrs[0]), addrs_cmp);
-	qsort(functions, functions_cnt, sizeof(functions[0]), functions_cmp);
-
-	/*
-	 * Let's got through all collected functions and filter
-	 * out those that are not in ftrace.
-	 */
-	for (i = 0; i < functions_cnt; i++) {
-		struct elf_function *func = &functions[i];
-		/*
-		 * For vmlinux image both addrs[x] and functions[x]::addr
-		 * values are final address and are comparable.
-		 *
-		 * For kernel module addrs[x] is final address, but
-		 * functions[x]::addr is relative address within section
-		 * and needs to be relocated by adding sh_addr.
-		 */
-		if (kmod)
-			func->addr += func->sh_addr;
-
-		/* Make sure function is within ftrace addresses. */
-		if (is_ftrace_func(func, addrs, count)) {
-			/*
-			 * We iterate over sorted array, so we can easily skip
-			 * not valid item and move following valid field into
-			 * its place, and still keep the 'new' array sorted.
-			 */
-			if (i != functions_valid)
-				functions[functions_valid] = functions[i];
-			functions_valid++;
-		}
-	}
-
-	functions_cnt = functions_valid;
-	free(addrs);
-
-	if (btf_elf__verbose)
-		printf("Found %d functions!\n", functions_cnt);
-	return 0;
-}
-
 static struct elf_function *find_function(const struct btf_elf *btfe,
 					  const char *name)
 {
@@ -620,23 +372,8 @@ static int collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym,
 	return 0;
 }
 
-static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl,
-			   size_t sym_sec_idx)
-{
-	if (!fl->mcount_start &&
-	    !strcmp("__start_mcount_loc", elf_sym__name(sym, btfe->symtab))) {
-		fl->mcount_start = sym->st_value;
-		fl->mcount_sec_idx = sym_sec_idx;
-	}
-
-	if (!fl->mcount_stop &&
-	    !strcmp("__stop_mcount_loc", elf_sym__name(sym, btfe->symtab)))
-		fl->mcount_stop = sym->st_value;
-}
-
 static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars)
 {
-	struct funcs_layout fl = { };
 	Elf32_Word sym_sec_idx;
 	uint32_t core_id;
 	GElf_Sym sym;
@@ -650,7 +387,6 @@ static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars)
 			return -1;
 		if (collect_function(btfe, &sym, sym_sec_idx))
 			return -1;
-		collect_symbol(&sym, &fl, sym_sec_idx);
 	}
 
 	if (collect_percpu_vars) {
@@ -661,9 +397,11 @@ static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars)
 			printf("Found %d per-CPU variables!\n", percpu_var_cnt);
 	}
 
-	if (functions_cnt && setup_functions(btfe, &fl)) {
-		fprintf(stderr, "Failed to filter DWARF functions\n");
-		return -1;
+	if (functions_cnt) {
+		qsort(functions, functions_cnt, sizeof(functions[0]), functions_cmp);
+
+		if (btf_elf__verbose)
+			printf("Found %d functions!\n", functions_cnt);
 	}
 
 	return 0;
-- 
2.31.1

Re: linux-next failing build due to missing cubictcp_state symbol

[Martin KaFai Lau]

On Tue, May 04, 2021 at 08:41:47AM +0200, Jiri Slaby wrote:
> On 03. 05. 21, 12:08, Jiri Olsa wrote:
> > On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
> > > CCing pahole people.
> > > 
> > > On 03. 05. 21, 9:59, Jiri Slaby wrote:
> > > > On 03. 05. 21, 8:11, Jiri Slaby wrote:
> > > > > > > > > > looks like vfs_truncate did not get into BTF data,
> > > > > > > > > > I'll try to reproduce
> > > > > > 
> > > > > > _None_ of the functions are generated by pahole -J from
> > > > > > debuginfo on ppc64. debuginfo appears to be correct. Neither
> > > > > > pahole -J fs/open.o works correctly. collect_functions in
> > > > > > dwarves seems to be defunct on ppc64... "functions" array is
> > > > > > bogus (so find_function -- the bsearch -- fails).
> > > > > 
> > > > > It's not that bogus. I forgot an asterisk:
> > > > > > #0  find_function (btfe=0x100269f80, name=0x10024631c
> > > > > > "stream_open") at
> > > > > > /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
> > > > > > (gdb) p (*functions)@84
> > > > > > $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
> > > > > > = 75232, size = 72, sh_addr = 65536, generated = false}, {
> > > > > >      name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
> > > > > > size = 216, sh_addr = 65536, generated = false}, {
> > > > > >      name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
> > > > > > 80816, size = 232, sh_addr = 65536, generated = false}, {
> > > > > >      name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
> > > > > > 74304, size = 100, sh_addr = 65536, generated = false}, {
> > > > > ...
> > > > > >      name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
> > > > > > 72, sh_addr = 65536, generated = false}, {
> > > > > ...
> > > > > >      name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
> > > > > > 544, sh_addr = 65536, generated = false}}
> > > > > 
> > > > > The dot makes the difference, of course. The question is why is it
> > > > > there? I keep looking into it. Only if someone has an immediate
> > > > > idea...
> > > > 
> > > > Well, .vfs_truncate is in .text (and contains an ._mcount call). And
> > > > vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
> > > > excludes all functions without the ._mcount call, is_ftrace_func later
> > > > returns false for such functions and they are filtered before the BTF
> > > > processing.
> > > > 
> > > > Technically, get_vmlinux_addrs looks at a list of functions between
> > > > __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> > > > 
> > > > I don't know what the correct fix is (exclude .opd functions from the
> > > > filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
> > > > this too.
> > > 
> > > Attaching a patch for pahole which fixes the issue, but I have no idea
> > > whether it is the right fix at all.
> > 
> > hi,
> > we're considering to disable ftrace filter completely,
> > I guess that would solve this issue for ppc as well
> > 
> >    https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/
> 
> Right, the attached patch fixes it for me too.
Ah, I just noticed the attachment while replying an earlier message in
this thread.

Please feel free to add SOB to mine or
repost yours and toss mine.  Either way works for me.

Re: linux-next failing build due to missing cubictcp_state symbol

[Jiri Olsa]

On Wed, May 05, 2021 at 10:31:52PM -0700, Martin KaFai Lau wrote:
> On Tue, May 04, 2021 at 08:41:47AM +0200, Jiri Slaby wrote:
> > On 03. 05. 21, 12:08, Jiri Olsa wrote:
> > > On Mon, May 03, 2021 at 10:59:44AM +0200, Jiri Slaby wrote:
> > > > CCing pahole people.
> > > > 
> > > > On 03. 05. 21, 9:59, Jiri Slaby wrote:
> > > > > On 03. 05. 21, 8:11, Jiri Slaby wrote:
> > > > > > > > > > > looks like vfs_truncate did not get into BTF data,
> > > > > > > > > > > I'll try to reproduce
> > > > > > > 
> > > > > > > _None_ of the functions are generated by pahole -J from
> > > > > > > debuginfo on ppc64. debuginfo appears to be correct. Neither
> > > > > > > pahole -J fs/open.o works correctly. collect_functions in
> > > > > > > dwarves seems to be defunct on ppc64... "functions" array is
> > > > > > > bogus (so find_function -- the bsearch -- fails).
> > > > > > 
> > > > > > It's not that bogus. I forgot an asterisk:
> > > > > > > #0  find_function (btfe=0x100269f80, name=0x10024631c
> > > > > > > "stream_open") at
> > > > > > > /usr/src/debug/dwarves-1.21-1.1.ppc64/btf_encoder.c:350
> > > > > > > (gdb) p (*functions)@84
> > > > > > > $5 = {{name = 0x7ffff68e0922 ".__se_compat_sys_ftruncate", addr
> > > > > > > = 75232, size = 72, sh_addr = 65536, generated = false}, {
> > > > > > >      name = 0x7ffff68e019e ".__se_compat_sys_open", addr = 80592,
> > > > > > > size = 216, sh_addr = 65536, generated = false}, {
> > > > > > >      name = 0x7ffff68e0076 ".__se_compat_sys_openat", addr =
> > > > > > > 80816, size = 232, sh_addr = 65536, generated = false}, {
> > > > > > >      name = 0x7ffff68e0908 ".__se_compat_sys_truncate", addr =
> > > > > > > 74304, size = 100, sh_addr = 65536, generated = false}, {
> > > > > > ...
> > > > > > >      name = 0x7ffff68e0808 ".stream_open", addr = 65824, size =
> > > > > > > 72, sh_addr = 65536, generated = false}, {
> > > > > > ...
> > > > > > >      name = 0x7ffff68e0751 ".vfs_truncate", addr = 73392, size =
> > > > > > > 544, sh_addr = 65536, generated = false}}
> > > > > > 
> > > > > > The dot makes the difference, of course. The question is why is it
> > > > > > there? I keep looking into it. Only if someone has an immediate
> > > > > > idea...
> > > > > 
> > > > > Well, .vfs_truncate is in .text (and contains an ._mcount call). And
> > > > > vfs_truncate is in .opd (w/o an ._mcount call). Since setup_functions
> > > > > excludes all functions without the ._mcount call, is_ftrace_func later
> > > > > returns false for such functions and they are filtered before the BTF
> > > > > processing.
> > > > > 
> > > > > Technically, get_vmlinux_addrs looks at a list of functions between
> > > > > __start_mcount_loc and __stop_mcount_loc and considers only the listed.
> > > > > 
> > > > > I don't know what the correct fix is (exclude .opd functions from the
> > > > > filter?). Neither why cross compiler doesn't fail, nor why ebi v2 avoids
> > > > > this too.
> > > > 
> > > > Attaching a patch for pahole which fixes the issue, but I have no idea
> > > > whether it is the right fix at all.
> > > 
> > > hi,
> > > we're considering to disable ftrace filter completely,
> > > I guess that would solve this issue for ppc as well
> > > 
> > >    https://lore.kernel.org/bpf/20210501001653.x3b4rk4vk4iqv3n7@kafai-mbp.dhcp.thefacebook.com/
> > 
> > Right, the attached patch fixes it for me too.
> Ah, I just noticed the attachment while replying an earlier message in
> this thread.
> 
> Please feel free to add SOB to mine or
> repost yours and toss mine.  Either way works for me.
> 

I think this patch is missing the same removal I just commented
on your patch.. either way is ok for me

jirka