lkml.org 
[lkml]   [2011]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [tip:perf/core] perf: Ignore non-sampling overflows
    On 29.06.11 06:50:55, Peter Zijlstra wrote:
    > On Wed, 2011-06-29 at 12:37 +0200, Robert Richter wrote:
    >
    > > I looked at the interrupt handlers. The events are always determined
    > > from a per-cpu array:
    > >
    > > cpuc = &__get_cpu_var(cpu_hw_events);
    > > ...
    > > event = cpuc->events[idx];
    > >
    > > In case of interrupts the event should then always be a hw event (or
    > > uninitialized). Even if the interrupt was triggered by a different
    > > source, it would always be mapped to the same event and the check
    > > is_sampling_event() would be meaningless.
    >
    > I'm probably not quite getting what you mean, but how is
    > is_sampling_event() meaningless? the INT bit is enabled for _all_
    > events, whether they were configured as a sampling event or not.

    Aren't all events that are mapped to counters via cpu_hw_events always
    sampling events? Then, when calling perf_event_overflow() from an
    interrupt handler there are no other events than sampling events.

    >
    > Its just that for !sampling events we shouldn't attempt to generate any
    > output.

    If attr.sample_type is null, there is no output to generate. Better
    use this instead of attr.sample_type in is_sampling_event()?
    perf_event_overflow() could be used then to generate output also for
    samples where no period is specified.

    >
    > > There are other occurrences of perf_event_overflow() in
    > > kernel/events/core.c for events of type PERF_TYPE_SOFTWARE. These
    > > events are initialized with sample_period set and a check would always
    > > be true too.
    >
    > I'm failing to see what you mean, where do we always set
    > event->attr.sample_period for software events?

    Hmm, I read the code wrong and the check in perf_event_overflow()
    might be needed for swevents.

    >
    > > For both cases I stil don't see a reason for the check.
    >
    > You're going to have to spell things out for me, I'm really not getting
    > your argument.

    I was thinking about to change this check and haven't seen cases for
    that the check is for. What would happen if the check isn't there and
    perf_event_overflow() is called from the interrupt handler?

    >
    > > Anyway, would the following extentension of the check above ok?
    > >
    > > if (unlikely(!is_sampling_event(event) && !event->attr.sample_type))
    > > ...
    > >
    > > With no bits set in attr.sample_type the sample would be empty and
    > > nothing to report. Now, with this change, samples that have data to
    > > report wouldn't be dropped anymore.
    >
    > Also, could you explain in what way data is dropped? Where do
    > non-sampling events need to write sample data?

    I stumbled over this while rebasing my perf ibs patches:

    http://git.kernel.org/?p=linux/kernel/git/rric/oprofile.git;a=shortlog;h=refs/heads/perf-ibs

    Hope I could explain this to you better now.

    -Robert

    --
    Advanced Micro Devices, Inc.
    Operating System Research Center



    \
     
     \ /
      Last update: 2011-06-29 16:13    [W:0.028 / U:34.192 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site