Hi Arnaldo, Sorry for late reply. I was offline last week.
On Tue, 11 Nov 2014 10:02:29 -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Nov 11, 2014 at 01:24:38PM +0900, Namhyung Kim escreveu: >> Hi Peter, >> >> On Mon, Nov 10, 2014 at 9:11 PM, Peter Zijlstra <[email protected]> wrote: >> > On Mon, Nov 10, 2014 at 03:33:11PM +0900, Namhyung Kim wrote: >> >> Hmm.. I don't think it's specific to the minimal elf parser. The return >> >> value of dso__load_sym() is a number of symbols found so when it sees a >> >> dso with 0 symbols it'll fall back to the next option IMHO (not >> >> tested). Did you see a problem with the current code? >> > >> > So your patch: >> > >> > +++ b/tools/perf/util/symbol-minimal.c >> > @@ -335,6 +335,9 @@ int dso__load_sym(struct dso *dso, struct map *map >> > __maybe_unused, >> > unsigned char *build_id[BUILD_ID_SIZE]; >> > int ret; >> > >> > + if (dso->kernel) >> > + return 0; /* always use kallsyms */ >> > + >> > >> > changes the symbol-minimal.c file to add this exception. That is very >> > much specific to the minimal elf parser, or am I just seeing things? >> >> What minimal parser does here is just skip kernel dsos (vmlinux) since >> it didn't deal with all the details of parsing vmlinux currently. >> >> >> > >> > What I was saying, why not have a util/symbol.c change that disregards >> > all DSOs with 0 symbols in. >> >> The util/symbol.c doesn't need this because it can handle vmlinux >> reliably. So after reading symbol table, it'll use the dso if it > > symbol.c should not be able to handle anything, since the actual ELF > loader is in symbol-elf.c or simbol-minimal.c, no? Ah right, I was confused with symbol.c and symbol-elf.c - so I believe the generic logic in symbol.c already handles DSOs with 0 symbol. The problem was symbol-minimal.c didn't return 0 for the DSOs. > > I.e.: > > symbol.c::dso__load() > symbol.c::dso__load_kernel_sym() > symbol.c::dso__load_vmlinux() > > And then it heads into one of the ELF loaders, right now, without your > patch, I see, when the minimal loader is used: > > symbol-minimal.c::dso__load_sym() > > reads the build-id and if it works, returns 1, which is a bug, it should > return 0 in this case, possibly -1 if it doesn't read the build-id :-\ > > I.e. it should return 0, because that way it signals: "I can't read it, > thus no symbols were loaded, symbol.c: please go on looking for them > somewhere else, kallsyms perhaps?". Right. It's a bug for vmlinux. But by returning 0, it'll iterate over the candidate binaries in dso__load(). I'm afraid that this *might* overwrite the build-id of the DSO with different one. So I think it also needs to add a check for valid build-id. I'll update this patch with above. Thanks, Namhyung > > And this is the bug, probably. Now to look at your patches to see if > this is touched somehow... > >> actually contains symbols or fallback to next dso if it has 0 symbols. >> IOW it already disregards all dsos with 0 symbols in. > > See above. > >> -- >> Thanks, >> Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
