lkml.org 
[lkml]   [2018]   [Aug]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 0/2] x86/intel_rdt and perf/x86: Fix lack of coordination with perf
    From
    Date
    Hi Peter and Tony,

    On 8/8/2018 12:51 AM, Peter Zijlstra wrote:
    > On Tue, Aug 07, 2018 at 03:47:15PM -0700, Reinette Chatre wrote:
    >>> FWIW, how long is that IRQ disabled section? It looks like something
    >>> that could be taking a bit of time. We have these people that care about
    >>> IRQ latency.
    >>
    >> We work closely with customers needing low latency as well as customers
    >> needing deterministic behavior.
    >>
    >> This measurement is triggered by the user as a validation mechanism of
    >> the pseudo-locked memory region after it has been created as part of
    >> system setup as well as during runtime if there are any concerns with
    >> the performance of an application that uses it.
    >>
    >> This measurement would thus be triggered before the sensitive workloads
    >> start - during system setup, or if an issue is already present. In
    >> either case the measurement is triggered by the administrator via debugfs.
    >
    > That does not in fact include the answer to the question. Also, it
    > assumes a competent operator (something I've found is not always true).

    My apologies, I did not intend to avoid your question. The main goal of
    this measurement is for an operator to test if a pseudo-locked region
    has been set up correctly. It is thus targeted for system setup time,
    before the IRQ sensitive workloads - some of which would use these
    memory regions - start.

    >>> - I don't much fancy people accessing the guts of events like that;
    >>> would not an inline function like:
    >>>
    >>> static inline u64 x86_perf_rdpmc(struct perf_event *event)
    >>> {
    >>> u64 val;
    >>>
    >>> lockdep_assert_irqs_disabled();
    >>>
    >>> rdpmcl(event->hw.event_base_rdpmc, val);
    >>> return val;
    >>> }
    >>>
    >>> Work for you?
    >>
    >> No. This does not provide accurate results. Implementing the above produces:
    >> pseudo_lock_mea-366 [002] .... 34.950740: pseudo_lock_l2: hits=4096
    >> miss=4
    >
    > But it being an inline function should allow the compiler to optimize
    > and lift the event->hw.event_base_rdpmc load like you now do manually.
    > Also, like Tony already suggested, you can prime that load just fine by
    > doing an extra invocation.
    >
    > (and note that the above function is _much_ simpler than
    > perf_event_read_local())

    Unfortunately I do not find this to be the case. When I implement
    x86_perf_rdpmc() _exactly_ as you suggest above and do the measurement like:

    l2_hits_before = x86_perf_rdpmc(l2_hit_event);
    l2_miss_before = x86_perf_rdpmc(l2_miss_event);
    l2_hits_before = x86_perf_rdpmc(l2_hit_event);
    l2_miss_before = x86_perf_rdpmc(l2_miss_event);
    /* read memory */
    l2_hits_after = x86_perf_rdpmc(l2_hit_event);
    l2_miss_after = x86_perf_rdpmc(l2_miss_event);


    Then the results are not accurate, neither are the consistently
    inaccurate to consider a constant adjustment:

    pseudo_lock_mea-409 [002] .... 194.322611: pseudo_lock_l2: hits=4100
    miss=0
    pseudo_lock_mea-412 [002] .... 195.520203: pseudo_lock_l2: hits=4096
    miss=3
    pseudo_lock_mea-415 [002] .... 196.571114: pseudo_lock_l2: hits=4097
    miss=3
    pseudo_lock_mea-422 [002] .... 197.629118: pseudo_lock_l2: hits=4097
    miss=3
    pseudo_lock_mea-425 [002] .... 198.687160: pseudo_lock_l2: hits=4096
    miss=3
    pseudo_lock_mea-428 [002] .... 199.744156: pseudo_lock_l2: hits=4096
    miss=2
    pseudo_lock_mea-431 [002] .... 200.801131: pseudo_lock_l2: hits=4097
    miss=2
    pseudo_lock_mea-434 [002] .... 201.858141: pseudo_lock_l2: hits=4097
    miss=2
    pseudo_lock_mea-437 [002] .... 202.917168: pseudo_lock_l2: hits=4096
    miss=2

    I was able to test Tony's theory and replacing the reading of the
    "after" counts with a direct rdpmcl() improve the results. What I mean
    is this:

    l2_hit_pmcnum = x86_perf_rdpmc_ctr_get(l2_hit_event);
    l2_miss_pmcnum = x86_perf_rdpmc_ctr_get(l2_miss_event);
    l2_hits_before = x86_perf_rdpmc(l2_hit_event);
    l2_miss_before = x86_perf_rdpmc(l2_miss_event);
    l2_hits_before = x86_perf_rdpmc(l2_hit_event);
    l2_miss_before = x86_perf_rdpmc(l2_miss_event);
    /* read memory */
    rdpmcl(l2_hit_pmcnum, l2_hits_after);
    rdpmcl(l2_miss_pmcnum, l2_miss_after);

    I did not run my full tests with the above but a simple read of 256KB
    pseudo-locked memory gives:
    pseudo_lock_mea-492 [002] .... 372.001385: pseudo_lock_l2: hits=4096
    miss=0
    pseudo_lock_mea-495 [002] .... 373.059748: pseudo_lock_l2: hits=4096
    miss=0
    pseudo_lock_mea-498 [002] .... 374.117027: pseudo_lock_l2: hits=4096
    miss=0
    pseudo_lock_mea-501 [002] .... 375.182864: pseudo_lock_l2: hits=4096
    miss=0
    pseudo_lock_mea-504 [002] .... 376.243958: pseudo_lock_l2: hits=4096
    miss=0

    We thus seem to be encountering the issue Tony predicted where the
    memory being tested is evicting the earlier measurement code and data.

    >>> - native_read_pmc(); are you 100% sure this code only ever runs on
    >>> native and not in some dodgy virt environment?
    >>
    >> My understanding is that a virtual environment would be a customer of a
    >> RDT allocation (cache or memory bandwidth). I do not see if/where this
    >> is restricted though - I'll move to rdpmcl() but the usage of a cache
    >> allocation feature like this from a virtual machine needs more
    >> investigation.
    >
    > I can imagine that hypervisors that allow physical partitioning could
    > allow delegating the rdt crud to their guests when they 'own' a full
    > socket or whatever the domain is for this.

    I am using rdpmcl() now

    >
    >> Will do. I created the following helper function that can be used after
    >> interrupts are disabled:
    >>
    >> static inline int perf_event_error_state(struct perf_event *event)
    >> {
    >> int ret = 0;
    >> u64 tmp;
    >>
    >> ret = perf_event_read_local(event, &tmp, NULL, NULL);
    >> if (ret < 0)
    >> return ret;
    >>
    >> if (event->attr.pinned && event->oncpu != smp_processor_id())
    >> return -EBUSY;
    >>
    >> return ret;
    >> }
    >
    > Nah, stick the test in perf_event_read_local(), that actually needs it.

    I will keep this outside for now since it does not seem as though
    perf_event_read_local() works well for this use case.


    >>> Also, while you disable IRQs, your fancy pants loop is still subject to
    >>> NMIs that can/will perturb your measurements, how do you deal with
    >>> those?
    >
    >> Customers interested in this feature are familiar with dealing with them
    >> (and also SMIs). The user space counterpart is able to detect such an
    >> occurrence.
    >
    > You're very optimistic about your customers capabilities. And this might
    > be true for the current people you're talking to, but once this is
    > available and public, joe monkey will have access and he _will_ screw it
    > up.

    I think this ventures into an area that is an existing issue for all
    workloads that fall into this category, not unique to this.

    >
    >> Please note that if an NMI arrives it would be handled with the
    >> currently active cache capacity bitmask so none of the pseudo-locked
    >> memory will be evicted since no capacity bitmask overlaps with the
    >> pseudo-locked region.
    >
    > So exceptions change / have their own bitmask?

    My understanding is that interrupts are run with the current active
    capacity bitmask. The capacity bitmasks specify which portions of cache
    a cpu and/or task may allocate into. No capacity bitmask would overlap
    with the pseudo-locked region and thus no cpu or task would be able to
    evict the data from the pseudo-locked region. Even if later interrupts
    do obtain their own class of service with its own capacity bitmasks -
    these btimasks would still not be allowed to overlap with the
    pseudo-locked region.

    Reinette


    \
     
     \ /
      Last update: 2018-08-08 19:33    [W:2.649 / U:0.812 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site