lkml.org 
[lkml]   [2023]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH V2] perf top: Use evsel's cpus to replace user_requested_cpus
    On Tue, Dec 12, 2023 at 2:12 PM Ian Rogers <irogers@google.com> wrote:
    >
    > On Tue, Dec 12, 2023 at 1:25 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
    > >
    > >
    > >
    > > On 2023-12-12 3:37 p.m., Ian Rogers wrote:
    > > > On Tue, Dec 12, 2023 at 11:39 AM <kan.liang@linux.intel.com> wrote:
    > > >>
    > > >> From: Kan Liang <kan.liang@linux.intel.com>
    > > >>
    > > >> perf top errors out on a hybrid machine
    > > >> $perf top
    > > >>
    > > >> Error:
    > > >> The cycles:P event is not supported.
    > > >>
    > > >> The perf top expects that the "cycles" is collected on all CPUs in the
    > > >> system. But for hybrid there is no single "cycles" event which can cover
    > > >> all CPUs. Perf has to split it into two cycles events, e.g.,
    > > >> cpu_core/cycles/ and cpu_atom/cycles/. Each event has its own CPU mask.
    > > >> If a event is opened on the unsupported CPU. The open fails. That's the
    > > >> reason of the above error out.
    > > >>
    > > >> Perf should only open the cycles event on the corresponding CPU. The
    > > >> commit ef91871c960e ("perf evlist: Propagate user CPU maps intersecting
    > > >> core PMU maps") intersect the requested CPU map with the CPU map of the
    > > >> PMU. Use the evsel's cpus to replace user_requested_cpus.
    > > >>
    > > >> The evlist's threads are also propagated to the evsel's threads in
    > > >> __perf_evlist__propagate_maps(). For a system-wide event, perf appends
    > > >> a dummy event and assign it to the evsel's threads. For a per-thread
    > > >> event, the evlist's thread_map is assigned to the evsel's threads. The
    > > >> same as the other tools, e.g., perf record, using the evsel's threads
    > > >> when opening an event.
    > > >>
    > > >> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
    > > >> Closes: https://lore.kernel.org/linux-perf-users/ZXNnDrGKXbEELMXV@kernel.org/
    > > >> Reviewed-by: Ian Rogers <irogers@google.com>
    > > >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
    > > >> ---
    > > >>
    > > >> Changes since V1:
    > > >> - Update the description
    > > >> - Add Reviewed-by from Ian
    > > >
    > > > Thanks Kan, quick question. Does "perf top" on hybrid ask the user to
    > > > select between the cycles event on cpu_atom and cpu_core?
    > >
    > > Yes, but the event doesn't include the PMU information.
    > > We probably need a follow up patch to append the PMU name.
    > >
    > > Available samples
    > > 385 cycles:P
    > >
    > > 903 cycles:P
    >
    > Thanks and agreed, it isn't possible to tell which is which PMU/CPU
    > type at the moment. I tried the patch with perf top --stdio, there
    > wasn't a choice of event and I can't tell what counter is being
    > displayed. When I quit I also see:
    > ```
    > exiting.
    > corrupted double-linked list
    > Aborted (core dumped)
    > ```
    > but I wasn't able to repro this on a debuggable binary/system.
    >
    > If my memory serves there was a patch where perf top was showing >1
    > event. It would be nice here to do some kind of hybrid merging rather
    > than having to view each PMU's top separately.

    Using event groups, but I noticed you removed the --group option.
    Maybe perf top can just use `{ ... }` notation for explicit grouping,
    but it might be implicit like in the hybrid case.

    Thanks,
    Namhyung

    \
     
     \ /
      Last update: 2023-12-13 02:07    [W:7.700 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site