lkml.org 
[lkml]   [2010]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 2/3 v3] perf: Implement Nehalem uncore pmu
    From
    Date
    Thanks for your great comments.

    Let me read it carefully, and then reply back.

    Lin Ming

    On Fri, 2010-12-10 at 07:46 +0800, Stephane Eranian wrote:
    > Hi,
    >
    > So I have tested this patch a bit on WSM and as I expected there
    > are issues with sampling.
    >
    > When HT is on, both siblings CPUs get the interrupt. The HW does not
    > allow you to only point interrupts to a single HT thread (CPU).
    >
    > I did verify that indeed both threads get the interrupt and that you have a
    > race condition. Both sibling CPUs stop uncore, get the status. They may get
    > the same overflow status. Both will pass the uncore->active_mask because
    > it's shared among siblings cores. Thus, you have a race for the whole
    > interrupt handler execution.
    >
    > You need some serialization in there. But the patch does not address this.
    > The problem is different from the back-to-back interrupt issue that
    > Don worked on.
    > The per-cpu marked/handled trick cannot work to avoid this problem.
    >
    > You cannot simply say "the lowest indexed" CPU of a sibling pair
    > handles the interrupt
    > because you don't know if this in an uncore intr, core interrupt or
    > something else. You
    > need to check. That means each HT thread needs to check uncore
    > ovfl_status. IF the
    > status is zero, then return. Otherwise, you need to do a 2nd level
    > check before you can
    > execute the handler. You need to know if the sibling CPU has already
    > "consumed" that
    > interrupt.
    >
    > I think you need some sort of generation counter per physical core and
    > per HT thread.
    > On interrupt, you could do something along the line of:
    > if (mycpu->intr_count == mysibling->intr_count) {
    > then mycpu->intr_count++
    > execute intr_handler()
    > } else {
    > mycpu->intr_count++
    > return;
    > }
    > Of course, the above needs some atomicity and ad locking (but I don't
    > think you can
    > use locks in NMI context).
    >
    > This makes me wonder if vectoring uncore to NMI is really needed,
    > given you cannot
    > correlated to an IP, incl. a kernel IP. If we were to vector to a
    > dedicated (lower prio)
    > vector, then we could use the trick of saying the lowest indexed CPU in a pair
    > handles the interrupt (because we would already know this is an uncore
    > interrupt).
    > This would be much simpler. Price: not samples in kernel's critical
    > section. But those
    > are useless anyway with uncore events.
    >
    > - uncore_get_status().
    > PERF_GLOBAL_STATUS contains more than overflow
    > status, bit 61-63 need to be masked off.
    >
    > - uncore_pmu_cpu_prepare()
    > I don't understand the x86_max_cores < 2 test. If you run your
    > NHM/WSM is single core mode, you still have uncore to deal with
    > thus, you need cpuc->intel_uncore initialized, don't you?
    >
    > - I assume that the reason the uncore->refcnt management is not
    > using atomic ops because the whole CPU hotplug is serialized, right?




    \
     
     \ /
      Last update: 2010-12-10 09:29    [W:2.646 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site