Re: [PATCH] perf: Add match_mode support for --symfs option

[duchangbin <changbin.du@huawei.com>]

On Sat, Feb 28, 2026 at 05:26:54PM -0800, Ian Rogers wrote:
> On Sat, Feb 28, 2026 at 2:06 AM duchangbin <changbin.du@huawei.com> wrote:
> >
> > After careful consideration, naming the new option as 'layout' is more
> > appropriate than 'match_mode', as it better aligns with the conventions
> > of technical terminology.
> >
> > I'll send patch V2 for this change.
> 
> Sgtm, some nits below.
>
[snip]
> > On Fri, Feb 27, 2026 at 11:13:40AM +0000, Changbin Du wrote:
> > > diff --git a/tools/perf/Documentation/tips.txt b/tools/perf/Documentation/tips.txt
> > > index 3fee9b2a88ea..268316596ba0 100644
> > > --- a/tools/perf/Documentation/tips.txt
> > > +++ b/tools/perf/Documentation/tips.txt
> > > @@ -11,7 +11,8 @@ Search options using a keyword: perf report -h <keyword>
> > >  Use parent filter to see specific call path: perf report -p <regex>
> > >  List events using substring match: perf list <keyword>
> > >  To see list of saved events and attributes: perf evlist -v
> > > -Use --symfs <dir> if your symbol files are in non-standard locations
> > > +Use --symfs <dir>[,basename] if your symbol files are in non-standard locations.
> > > + Use ',basename' when debug files are in a flat directory structure.
> 
> Should the documentation also mention ",path" ?
>
I changed it to --symfs <dir>[,layout] as below in patch V2.

-Use --symfs <dir> if your symbol files are in non-standard locations
+Use --symfs <dir>[,layout] if your symbol files are in non-standard locations.

> > >  To see callchains in a more compact form: perf report -g folded
> > >  To see call chains by final symbol taking CPU time (bottom up) use perf report -G
> > >  Show individual samples with: perf script
> > > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> > > index 9c27bb30b708..f4ce70998423 100644
> > > --- a/tools/perf/builtin-annotate.c
> > > +++ b/tools/perf/builtin-annotate.c
> > > @@ -744,8 +744,7 @@ int cmd_annotate(int argc, const char **argv)
> > >                       &annotate.group_set,
> > >                       "Show event group information together"),
> > >       OPT_STRING('C', "cpu", &annotate.cpu_list, "cpu", "list of cpus to profile"),
> > > -     OPT_CALLBACK(0, "symfs", NULL, "directory",
> > > -                  "Look for files with symbols relative to this directory",
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > >                    symbol__config_symfs),
> > >       OPT_BOOLEAN(0, "source", &annotate_opts.annotate_src,
> > >                   "Interleave source code with assembly code (default)"),
> > > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> > > index 59bf1f72d12e..7b7cc3c80dfd 100644
> > > --- a/tools/perf/builtin-diff.c
> > > +++ b/tools/perf/builtin-diff.c
> > > @@ -1280,8 +1280,7 @@ static const struct option options[] = {
> > >       OPT_STRING_NOEMPTY('t', "field-separator", &symbol_conf.field_sep, "separator",
> > >                  "separator for columns, no spaces will be added between "
> > >                  "columns '.' is reserved."),
> > > -     OPT_CALLBACK(0, "symfs", NULL, "directory",
> > > -                  "Look for files with symbols relative to this directory",
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > >                    symbol__config_symfs),
> > >       OPT_UINTEGER('o', "order", &sort_compute, "Specify compute sorting."),
> > >       OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
> > > diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> > > index 7f3068264568..838471edb9e5 100644
> > > --- a/tools/perf/builtin-kwork.c
> > > +++ b/tools/perf/builtin-kwork.c
> > > @@ -2423,8 +2423,8 @@ int cmd_kwork(int argc, const char **argv)
> > >                   "Display call chains if present"),
> > >       OPT_UINTEGER(0, "max-stack", &kwork.max_stack,
> > >                  "Maximum number of functions to display backtrace."),
> > > -     OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
> > > -                 "Look for files with symbols relative to this directory"),
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > > +                  symbol__config_symfs),
> > >       OPT_STRING(0, "time", &kwork.time_str, "str",
> > >                  "Time span for analysis (start,stop)"),
> > >       OPT_STRING('C', "cpu", &kwork.cpu_list, "cpu",
> > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > > index 1b4ba85ee019..e2912217153a 100644
> > > --- a/tools/perf/builtin-probe.c
> > > +++ b/tools/perf/builtin-probe.c
> > > @@ -597,8 +597,8 @@ __cmd_probe(int argc, const char **argv)
> > >       OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
> > >                   "Enable kernel symbol demangling"),
> > >       OPT_BOOLEAN(0, "cache", &probe_conf.cache, "Manipulate probe cache"),
> > > -     OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
> > > -                "Look for files with symbols relative to this directory"),
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > > +                  symbol__config_symfs),
> > >       OPT_CALLBACK(0, "target-ns", NULL, "pid",
> > >                    "target pid for namespace contexts", opt_set_target_ns),
> > >       OPT_BOOLEAN(0, "bootconfig", &probe_conf.bootconfig,
> > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > > index 3b81f4b3dc49..e0d81a8cc3bb 100644
> > > --- a/tools/perf/builtin-report.c
> > > +++ b/tools/perf/builtin-report.c
> > > @@ -1416,8 +1416,7 @@ int cmd_report(int argc, const char **argv)
> > >                  "columns '.' is reserved."),
> > >       OPT_BOOLEAN('U', "hide-unresolved", &symbol_conf.hide_unresolved,
> > >                   "Only display entries resolved to a symbol"),
> > > -     OPT_CALLBACK(0, "symfs", NULL, "directory",
> > > -                  "Look for files with symbols relative to this directory",
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > >                    symbol__config_symfs),
> > >       OPT_STRING('C', "cpu", &report.cpu_list, "cpu",
> > >                  "list of cpus to profile"),
> > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > > index 3f509cfdd58c..741e9894490c 100644
> > > --- a/tools/perf/builtin-sched.c
> > > +++ b/tools/perf/builtin-sched.c
> > > @@ -4879,8 +4879,8 @@ int cmd_sched(int argc, const char **argv)
> > >                   "Display call chains if present (default on)"),
> > >       OPT_UINTEGER(0, "max-stack", &sched.max_stack,
> > >                  "Maximum number of functions to display backtrace."),
> > > -     OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
> > > -                 "Look for files with symbols relative to this directory"),
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > > +                  symbol__config_symfs),
> > >       OPT_BOOLEAN('s', "summary", &sched.summary_only,
> > >                   "Show only syscall summary with statistics"),
> > >       OPT_BOOLEAN('S', "with-summary", &sched.summary,
> > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > > index 7c743a303507..4fe79971c97f 100644
> > > --- a/tools/perf/builtin-script.c
> > > +++ b/tools/perf/builtin-script.c
> > > @@ -4074,8 +4074,7 @@ int cmd_script(int argc, const char **argv)
> > >                  "file", "kallsyms pathname"),
> > >       OPT_BOOLEAN('G', "hide-call-graph", &no_callchain,
> > >                   "When printing symbols do not display call chain"),
> > > -     OPT_CALLBACK(0, "symfs", NULL, "directory",
> > > -                  "Look for files with symbols relative to this directory",
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > >                    symbol__config_symfs),
> > >       OPT_CALLBACK('F', "fields", NULL, "str",
> > >                    "comma separated output fields prepend with 'type:'. "
> > > diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> > > index f8b49d69e9a5..b071c66b8dc4 100644
> > > --- a/tools/perf/builtin-timechart.c
> > > +++ b/tools/perf/builtin-timechart.c
> > > @@ -1951,8 +1951,7 @@ int cmd_timechart(int argc, const char **argv)
> > >       OPT_CALLBACK('p', "process", NULL, "process",
> > >                     "process selector. Pass a pid or process name.",
> > >                      parse_process),
> > > -     OPT_CALLBACK(0, "symfs", NULL, "directory",
> > > -                  "Look for files with symbols relative to this directory",
> > > +     OPT_CALLBACK(0, "symfs", NULL, "directory[,match_mode]", SYMFS_HELP,
> > >                    symbol__config_symfs),
> > >       OPT_INTEGER('n', "proc-num", &tchart.proc_num,
> > >                   "min. number of tasks to print"),
> > > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > > index 8662001e1e25..0cafa6e630e8 100644
> > > --- a/tools/perf/util/symbol.c
> > > +++ b/tools/perf/util/symbol.c
> > > @@ -66,6 +66,7 @@ struct symbol_conf symbol_conf = {
> > >       .time_quantum           = 100 * NSEC_PER_MSEC, /* 100ms */
> > >       .show_hist_headers      = true,
> > >       .symfs                  = "",
> > > +     .symfs_match_basename   = false,
> > >       .event_group            = true,
> > >       .inline_name            = true,
> > >       .res_sample             = 0,
> > > @@ -2491,16 +2492,41 @@ int symbol__config_symfs(const struct option *opt __maybe_unused,
> > >                        const char *dir, int unset __maybe_unused)
> > >  {
> > >       char *bf = NULL;
> > > +     char *match_mode_str;
> > >       int ret;
> > >
> > > -     symbol_conf.symfs = strdup(dir);
> > > -     if (symbol_conf.symfs == NULL)
> > > -             return -ENOMEM;
> > > +     match_mode_str = strchr(dir, ',');
> 
> I wonder if using `strstr` or `ends_with` for ",basename" and ",path"
> would work better than `strchr` for ','. That way if a path contains a
> comma (which is very unusual), the old behavior will happen.
>

I fixed by:
1. Using strrchr() to find the last comma instead of first
2. Only treating it as a layout specifier if suffix is "flat" or "hierarchy"

This handles all cases correctly:
- /path,flat → suffix is "flat" → split → symfs=/path, layout=flat
- /path,hierarchy → suffix is "hierarchy" → split → symfs=/path, layout=hierarchy
- ./my,symfs/ → suffix is "symfs/" (not valid) → full path as symfs
- /some,path/file → suffix is "file" (not valid) → full path as symfs

> Thanks,
> Ian
> 

-- 
Cheers,
Changbin Du