Hi,

Given the way the code is structured today, what you are seeing
is expected. Even though you are in one-shot mode with IOC_REFRESH
the new sampling period is not taken into account until the next next period.
The reason is that by the time you get the notification, the next period has
already been reloaded into the counter, but the counter is stopped. When you
issue IOC_REFRESH, it simply re-enables the counter but does not reload
the period.

I think this is a bug. You'd expect that given the counter is stopped when you
get notified, you could make changes to it and those changes would be taken
into account when you re-active the counter. In other words, IOC_REFRESH
should also reload the period (or anything else you might have changed). I will
take a look at this this week.



On Mon, Nov 15, 2010 at 3:06 AM, heechul Yun <heec...@illinois.edu> wrote:
> It seems that still there's a problem. The problem is the changed period,
> using ioctl(fd, PERF_EVENT_IOC_PERIOD, &new), does not takes effect on the
> next overflow but on the next next overflow.
> What I did is as follows:  on every overflow (1) print the current counter
> value, and (2) set the new sampling period (=previous sampling period +
> 10000000) and rearm it.
> With initial sample_period = 10000000ULL, the expected behavior was
> something like the below.
> 1 | value = 10000000 | new_period = 20000000
> 2 | value = 30000000 | new_period = 30000000
> 3 | value = 60000000 | new_period = 40000000
> 4 | value = 100000000 | new_period = 50000000
> 5 | value = 150000000 | new_period = 60000000
> The actual was, however, as follows:
> 1 | value = 10000000 | new_period = 20000000
> 2 | value = 20000000 | new_period = 30000000
> 3 | value = 40000000 | new_period = 40000000
> 4 | value = 70000000 | new_period = 50000000
> 5 | value = 110000000 | new_period = 60000000
> As you can see, the first ioctl, new_period=20000000, took effect on 3rd
> signal not the 2nd signal handling. Likewise, the second ioctl,
> new_period=30000000, took effect on 4th signal handling.
> Is this expected behavior? if not, how can this be corrected?
> The code I used is slightly modified notify_group.c of libpfm4 as shown in
> the following.
> --< notify_group.c >--
> #define SMPL_PERIOD 10000000ULL
> static void
> sigio_handler(int n, struct siginfo *info, struct sigcontext *sc)
> {
>         ...
>         notification_received++;
>         read(info->si_fd, values, sizeof(values));
> printf("%d | value = %lld | new_period = %lld\n", notification_received,
> values[0], new_period);
> new_period = SMPL_PERIOD * (notification_received + 1);
> ret = ioctl(info->si_fd, PERF_EVENT_IOC_PERIOD, &new_period);
> ret = ioctl(info->si_fd, PERF_EVENT_IOC_REFRESH, 1);
> }
> On Fri, Nov 12, 2010 at 9:40 AM, stephane eranian <eran...@googlemail.com>
> wrote:
>>
>> On Fri, Nov 12, 2010 at 4:36 PM, heechul Yun <heec...@illinois.edu> wrote:
>> >
>> >
>> > On Fri, Nov 12, 2010 at 8:41 AM, stephane eranian
>> > <eran...@googlemail.com>
>> > wrote:
>> >>
>> >> On Fri, Nov 12, 2010 at 2:22 PM, heechul Yun <heec...@illinois.edu>
>> >> wrote:
>> >> > It was because of a bug in kernel/perf_event.c  as shown in the
>> >> > following.
>> >> > static int perf_event_period(struct perf_event *event, u64 __user
>> >> > *arg)
>> >> > {
>> >> >        ...
>> >> > size = copy_from_user(&value, arg, sizeof(value));
>> >> > if (size != sizeof(value))  // <--------- bug.  should be: if (size)
>> >> > return
>> >> > -EFAULT
>> >> > return -EFAULT;
>> >> >
>> >>
>> >> Ok, I will post a file for this on LKML.
>> >> Unless you want to do it.
>> >>
>> >
>> > It seems that it is already fixed in the latest 2.6.36 release. All pre
>> > 2.6.36 (2.6.35 - 2.6.32 at least) had the bug.
>>
>> That sounds familiar. Yes, now I remember seeing a fix about
>> copy_from_user().
>> Thanks for tracking this down on your side anyway.
>>
>>
>> > Heechul
>> >
>> >> > I found this bug has been there in all versions before it was finally
>> >> > fixed
>> >> > in 2.6.36.
>> >> > Heechul
>> >> >
>> >> > On Thu, Nov 11, 2010 at 9:23 PM, heechul Yun <heec...@illinois.edu>
>> >> > wrote:
>> >> >>
>> >> >> Hi,
>> >> >> I wanted to adjust sampling period using PERF_EVENT_IOC_PERIOD
>> >> >> ioctl.
>> >> >> I modified the notify_group to change the period at runtime as
>> >> >> follows.
>> >> >> diff --git a/perf_examples/notify_group.c
>> >> >> b/perf_examples/notify_group.c
>> >> >> index 76869b0..d8ff5e1 100644
>> >> >> --- a/perf_examples/notify_group.c
>> >> >> +++ b/perf_examples/notify_group.c
>> >> >> @@ -54,6 +54,7 @@ sigio_handler(int n, struct siginfo *info, struct
>> >> >> sigcontext *sc)
>> >> >>        struct perf_event_mmap_page *hdr;
>> >> >>        struct perf_event_header ehdr;
>> >> >>        uint64_t ip;
>> >> >> +       uint64_t new_period;
>> >> >>        int id, ret;
>> >> >>
>> >> >>        id = perf_fd2event(fds, num_fds, info->si_fd);
>> >> >> @@ -87,6 +88,11 @@ skip:
>> >> >>        /*
>> >> >>         * rearm the counter for one more shot
>> >> >>         */
>> >> >> +       new_period = SMPL_PERIOD * 2 ;
>> >> >> +       ret = ioctl(info->si_fd, PERF_EVENT_IOC_PERIOD, new_period);
>> >> >> +       if (ret == -1)
>> >> >> +               err(1, "cannot set");
>> >> >> +
>> >> >>        ret = ioctl(info->si_fd, PERF_EVENT_IOC_REFRESH, 1);
>> >> >>        if (ret == -1)
>> >> >>                err(1, "cannot refresh");
>> >> >> It, however, failed to do so as belows.
>> >> >> $ ./notify_group
>> >> >> i=0 disabled=0
>> >> >> i=1 disabled=1
>> >> >> i=2 disabled=1
>> >> >> Notification 1: 0x8048f63 fd=3 PERF_COUNT_HW_CPU_CYCLES
>> >> >> notify_group: cannot set: Invalid argument
>> >> >>
>> >> >> Any idea?
>> >> >>
>> >> >> BTW, I found an error in perf_event.h of libpfm4 which prevents
>> >> >> compiling
>> >> >> the code.
>> >> >> The following diff is the fix.
>> >> >> diff --git a/include/perfmon/perf_event.h
>> >> >> b/include/perfmon/perf_event.h
>> >> >> index cfddef0..7f2889e 100644
>> >> >> --- a/include/perfmon/perf_event.h
>> >> >> +++ b/include/perfmon/perf_event.h
>> >> >> @@ -13,7 +13,7 @@
>> >> >>  */
>> >> >>  #ifndef _LINUX_PERF_EVENT_H
>> >> >>  #define _LINUX_PERF_EVENT_H
>> >> >> -
>> >> >> +#include <linux/types.h>
>> >> >>  #include <sys/types.h>
>> >> >>  #include <inttypes.h>
>> >> >>  #ifndef PR_TASK_PERF_EVENTS_DISABLE
>> >> >> @@ -236,7 +236,7 @@ typedef struct perf_event_attr {
>> >> >>  #define PERF_EVENT_IOC_DISABLE         _IO ('$', 1)
>> >> >>  #define PERF_EVENT_IOC_REFRESH         _IO ('$', 2)
>> >> >>  #define PERF_EVENT_IOC_RESET           _IO ('$', 3)
>> >> >> -#define PERF_EVENT_IOC_PERIOD          _IOW('$', 4, u64)
>> >> >> +#define PERF_EVENT_IOC_PERIOD          _IOW('$', 4, __u64)
>> >> >>  #define PERF_EVENT_IOC_SET_OUTPUT      _IO ('$', 5)
>> >> >>  #define PERF_EVENT_IOC_SET_FILTER      _IOW('$', 6, char *)
>> >> >>
>> >> >>
>> >> >> - Heechul
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > ------------------------------------------------------------------------------
>> >> > Centralized Desktop Delivery: Dell and VMware Reference Architecture
>> >> > Simplifying enterprise desktop deployment and management using
>> >> > Dell EqualLogic storage and VMware View: A highly scalable,
>> >> > end-to-end
>> >> > client virtualization framework. Read more!
>> >> > http://p.sf.net/sfu/dell-eql-dev2dev
>> >> > _______________________________________________
>> >> > perfmon2-devel mailing list
>> >> > perfmon2-devel@lists.sourceforge.net
>> >> > https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
>> >> >
>> >> >
>> >
>> >
>
>

------------------------------------------------------------------------------
Centralized Desktop Delivery: Dell and VMware Reference Architecture
Simplifying enterprise desktop deployment and management using
Dell EqualLogic storage and VMware View: A highly scalable, end-to-end
client virtualization framework. Read more!
http://p.sf.net/sfu/dell-eql-dev2dev
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel

Reply via email to