>-----Original Message-----
>From: Jiri Olsa [mailto:jo...@redhat.com]
>Sent: Saturday, November 4, 2017 6:25 AM
>To: Megha Dey <megha....@linux.intel.com>
>Cc: x...@kernel.org; linux-ker...@vger.kernel.org; linux-
>d...@vger.kernel.org; t...@linutronix.de; mi...@redhat.com;
>h...@zytor.com; andriy.shevche...@linux.intel.com;
>kstew...@linuxfoundation.org; Yu, Yu-cheng <yu-cheng...@intel.com>;
>Brown, Len <len.br...@intel.com>; gre...@linuxfoundation.org;
>pet...@infradead.org; a...@kernel.org;
>alexander.shish...@linux.intel.com; namhy...@kernel.org;
>vikas.shiva...@linux.intel.com; pombreda...@nexb.com;
>m...@kylehuey.com; b...@suse.de; Andrejczuk, Grzegorz
><grzegorz.andrejc...@intel.com>; Luck, Tony <tony.l...@intel.com>;
>cor...@lwn.net; Shankar, Ravi V <ravi.v.shan...@intel.com>; Dey, Megha
><megha....@intel.com>
>Subject: Re: [PATCH V0 2/3] perf/x86/intel/bm.c: Add Intel Branch
>Monitoring support
>
>On Fri, Nov 03, 2017 at 11:00:05AM -0700, Megha Dey wrote:
>
>SNIP
>
>> +
>> +static int intel_bm_event_nmi_handler(unsigned int cmd, struct
>> +pt_regs *regs) {
>> +    struct perf_event *event;
>> +    union bm_detect_status stat;
>> +    struct perf_sample_data data;
>> +    int i;
>> +    unsigned long x;
>> +
>> +    rdmsrl(BR_DETECT_STATUS_MSR, stat.raw);
>> +
>> +    if (stat.event) {
>> +            wrmsrl(BR_DETECT_STATUS_MSR, 0);
>> +            apic_write(APIC_LVTPC, APIC_DM_NMI);
>> +            /*
>> +             * Issue wake-up to corrresponding polling event
>> +             */
>> +            x = stat.ctrl_hit;
>> +            for_each_set_bit(i, &x, bm_num_counters) {
>> +                    event = bm_counter_owner[i];
>> +                    perf_sample_data_init(&data, 0, event-
>>hw.last_period);
>> +                    perf_event_overflow(event, &data, regs);
>
>hum, it's non sampling events only right? then you don't need any of the
>perf_sample_data stuff.. the perf_event_overflow call is basicaly nop

Yeah you are right. We were supporting sampling initially, forgot to get rid of 
this code.
Will change this in the next version.
>
>> +                    local64_inc(&event->count);
>> +                    atomic_set(&event->hw.bm_poll, POLLIN);
>> +                    event->pending_wakeup = 1;
>> +                    irq_work_queue(&event->pending);
>
>also this is for sampling events only
>
>seems like you only want to increment the event->count in here

We are currently working on a library similar to libperf which user space apps 
could make use of instead of perf command line monitoring. This code has been 
added so that the user can poll on when/if an interrupt is triggered and let 
the user know of its occurrence. If you think there is a better way of doing 
this, please let me know :)

>
>thanks,
>jirka
>
>> +            }
>> +            return NMI_HANDLED;
>> +    }
>> +    return NMI_DONE;
>> +}
>
>
>SNIP
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to