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

[Jiri Slaby <jirislaby@kernel.org>]

[-- 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