Re: [PATCH v2 0/5] kallsyms: Emit symbol for holes in text and fix weak function issue

[Zheng Yejian]

On 2024/12/14 03:31, Martin Kelly wrote:
> On Thu, 2024-12-12 at 17:52 +0800, Zheng Yejian wrote:
> > On 2024/12/11 04:49, Martin Kelly wrote:
> > > Zheng, do you plan to send a v3? I'd be happy to help out with this
> > > patch series if you'd like, as I'm hoping to get this issue
> > > resolved
> > > (though I am not an ftrace expert).
> >
> > Sorry to rely so late. Thanks for your feedback!
> >
> > Steve recently started a discussion of the issue in:
> >
> > https://lore.kernel.org/all/20241015100111.37adfb28@xxxxxxxxxxxxxxxxxx/
> > but there's no conclusion.
> >    
> > I can rebase this patch series and send a new version first, and
> > I'm also hoping to get more feedbacks and help to resolve the issue.
>
> Hi Zheng,
>
> I may have misunderstood, but based on the final message from Steven, I
> got the sense that the idea listed in that thread didn't work out and
> we should proceed with your current approach.

This patch set attempts to add length information of symbols to kallsyms,
which can then ensure that there is only one valid fentry within the range
of function length.

However, I think this patch set actually has some performance implications,
including：
  1. Adding hole symbols may cause the symbol table to grow significantly;
  2. When parsing fentry tables, excluding invalid fentries through kallsyms lookups
     may also increase boot or module load times slightly.

The direct cause of this issue is the wrong fentry being founded by ftrace_location(),
following the approach of "FTRACE_MCOUNT_MAX_OFFSET", narrowing down the search range
and re-finding may also solve this problem, demo patch like below (not
fully tested):

    diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
    index 9b17efb1a87d..7d34320ca9d1 100644
    --- a/kernel/trace/ftrace.c
    +++ b/kernel/trace/ftrace.c
    @@ -1678,8 +1678,11 @@ unsigned long ftrace_location(unsigned long ip)
                            goto out;
    
                    /* map sym+0 to __fentry__ */
    -               if (!offset)
    +               if (!offset) {
                            loc = ftrace_location_range(ip, ip + size - 1);
    +                       while (loc > ip && loc - ip > FTRACE_MCOUNT_MAX_OFFSET)
    +                               loc = ftrace_location_range(ip, loc - 1);
    +               }
            }

Steve, Peter, what do you think?

> Please consider me an interested party for this patch series, and let
> me know if there's anything I can do to help speed it along (co-
> develop, test, anything else). And of course, thanks very much for your
> work thus far!

--
Thanks,
Zheng Yejian