Re: [PATCH v5 0/2] perf: riscv: Preliminary Perf Event Support on RISC-V

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 24 Apr 2018 20:19:36 PDT (-0700), alankao@xxxxxxxxxxxxx wrote:
Hi Atish, Palmer,

On Tue, Apr 24, 2018 at 06:15:49PM -0700, Atish Patra wrote:
On 4/24/18 5:29 PM, Palmer Dabbelt wrote:
>On Tue, 24 Apr 2018 15:16:16 PDT (-0700), atish.patra@xxxxxxx wrote:
>>On 4/24/18 12:44 PM, Palmer Dabbelt wrote:
>>>On Tue, 24 Apr 2018 12:27:26 PDT (-0700), atish.patra@xxxxxxx wrote:
>>>>On 4/24/18 11:07 AM, Atish Patra wrote:
>>>>>On 4/19/18 4:28 PM, Alan Kao wrote:
>>>>>However, I got an rcu-stall for the test "47: Event times".
>>>>># ./perf test -v 47
>>>>Got it working. The test tries to attach the event to CPU0 which doesn't
>>>>exist in HighFive Unleashed. Changing it to cpu1 works.
>>>>
>>>>diff --git a/tools/perf/tests/event-times.c b/tools/perf/tests/event-times.c
>>>>index 1a2686f..eb11632f 100644
>>>>--- a/tools/perf/tests/event-times.c
>>>>+++ b/tools/perf/tests/event-times.c
>>>>@@ -113,9 +113,9 @@ static int attach__cpu_disabled(struct perf_evlist
>>>>*evlist)
>>>>           struct cpu_map *cpus;
>>>>           int err;
>>>>
>>>>-       pr_debug("attaching to CPU 0 as enabled\n");
>>>>+       pr_debug("attaching to CPU 1 as disabled\n");
>>>>
>>>>-       cpus = cpu_map__new("0");
>>>>+       cpus = cpu_map__new("1");
>>>>           if (cpus == NULL) {
>>>>                   pr_debug("failed to call cpu_map__new\n");
>>>>                   return -1;
>>>>@@ -142,9 +142,9 @@ static int attach__cpu_enabled(struct perf_evlist
>>>>*evlist)
>>>>           struct cpu_map *cpus;
>>>>           int err;
>>>>
>>>>-       pr_debug("attaching to CPU 0 as enabled\n");
>>>>+       pr_debug("attaching to CPU 1 as enabled\n");
>>>>
>>>>-       cpus = cpu_map__new("0");
>>>>+       cpus = cpu_map__new("1");
>>>>           if (cpus == NULL) {
>>>>                   pr_debug("failed to call cpu_map__new\n");
>>>>                   return -1;
>>>>
>>>>
>>>>Palmer,
>>>>Would it be better to officially document it somewhere that CPU0 doesn't
>>>>exist in the HighFive Unleashed board ?
>>>>I fear that there will be other standard tests/code path that may fail
>>>>because of inherent assumption of cpu0 presence.
>>>
>>>I think the best way to fix this is to just have BBL (or whatever the
>>>bootloader is) renumber the CPUs so they're contiguous and begin with 0.
>>
>>Do you mean BBL will update the device tree that kernel eventually parse
>>and set the hart id?
>>Sounds good to me unless it acts as a big hack in future boot loaders.
>
>Right now the machine-mode and supervisor-mode hart IDs are logically separate:
>the bootloader just provides the hart ID as a register argument when starting
>the kernel.

Yes.

  BBL already needs to enumerate the harts by looking through the
>device tree for various other reasons (at least to mask off the harts that
>Linux doesn't support), so it's not that much effort to just maintain a mapping
>from supervisor-mode hart IDs to machine-mode hart IDs.
>

But Linux also parses the device tree to get hart ID in
riscv_of_processor_hart(). This is used to setup the possible/present cpu
map in setup_smp().

Thus, Linux also need to see a device tree with cpu0-3 instead of cpu1-4.
Otherwise, present cpu map will be incorrect. Isn't it ?

>I have some patches floating around that do this, but appear to do it
>incorrectly enough that nothing boots so maybe I'm missing something that makes
>this complicated :).
>

Just a wild guess: May be the because of the above reason ;)


Thanks for the test and discussion.  It looks like am implementation issue from
Unleash, so ... is there anything I should fix and provide a further patch?

You're welcome to fix BBL if you want, but that's unrelated to this patch set. I'm going to look over the code again as soon as I get a chance to, thanks for submitting the patches!
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux