Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers
On 02/26/2014 12:29 AM, Peter Zijlstra wrote: On Tue, Feb 25, 2014 at 01:38:31PM -0800, Cody P Schafer wrote: On 02/25/2014 02:20 AM, Peter Zijlstra wrote: On Tue, Feb 25, 2014 at 02:33:26PM +1100, Michael Ellerman wrote: On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote: Export the swevent hrtimer helpers currently only used in events/core.c to allow the addition of architecture specific sw-like pmus. Peter, Ingo, can we get your ACK on this please? How are they used? I saw some usage in patch 9 or so; but its not explained anywhere. All patches have non-existent Changelogs and the few comments that are there are pretty hardware specific. So please do tell; what do you need this for? From this patch's change log: Export the swevent hrtimer helpers currently only used in events/core.c to allow the addition of architecture specific sw-like pmus. The key part here is "architecture specific sw-like pmus", where the announcement explains why these pmus are sw-like: I don't read announcements for crucial patch details; announcements are lost and therefore unimportant. And I'll be sure to elaborate further in the changelog next time (if I don't drop this change entirely). This is the first comment I've got on this particular patch. The counters supplied by these interfaces are continually counting and never need to be (and cannot be) disabled or enabled. They additionally do not generate any interrupts. This makes them in some regards similar to software counters, and as a result their implimentation shares some common code (which an initial patch exposes) with the sw counters. Essentially, these pmus just provide access to a big array of counters which don't generate interrupts, and are all 64bit (and assumed to never overflow). Rather than duplicate the code that we already have for managing timing when reading from counters that don't have interrupts (the functions that are exposed by this patch), I've reused it. So note that all the software counters generate interrupts in their own measuring domain. The hrtimer ones measure time and generate time based interrupts, the event based ones generate 'interrupts' on their events. What you have here is a hw pmu without interrupt capability. That's fine, they don't get to generate interrupt. We have plenty of those already. But what you propose to do is add interrupt in another domain entirely. That's not fine. Don't do that. Ok, so it looks like I misunderstood the need for an interrupt. The intention in using the swevent_hrtimer code was to enable setting up the events as frequency sampled. After taking another look at the gpci and 24x7 pmus, I'm forbidding sampling events anyhow in event init, so the timer code isn't even taken advantage of. I'll drop this patch in the next set. You also try and conceal this information; so you suck. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers
On Tue, Feb 25, 2014 at 01:38:31PM -0800, Cody P Schafer wrote: > On 02/25/2014 02:20 AM, Peter Zijlstra wrote: > >On Tue, Feb 25, 2014 at 02:33:26PM +1100, Michael Ellerman wrote: > >>On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote: > >>>Export the swevent hrtimer helpers currently only used in events/core.c > >>>to allow the addition of architecture specific sw-like pmus. > >> > >>Peter, Ingo, can we get your ACK on this please? > > > >How are they used? I saw some usage in patch 9 or so; but its not > >explained anywhere. All patches have non-existent Changelogs and the few > >comments that are there are pretty hardware specific. > > > >So please do tell; what do you need this for? > > From this patch's change log: > > >Export the swevent hrtimer helpers currently only used in events/core.c to > >allow the addition of architecture specific sw-like pmus. > > The key part here is "architecture specific sw-like pmus", where the > announcement explains why these pmus are sw-like: I don't read announcements for crucial patch details; announcements are lost and therefore unimportant. > >The counters supplied by these interfaces are continually counting and never > >need to be (and cannot be) disabled or enabled. They additionally do not > >generate any interrupts. This makes them in some regards similar to software > >counters, and as a result their implimentation shares some common code (which > >an initial patch exposes) with the sw counters. > > Essentially, these pmus just provide access to a big array of counters which > don't generate interrupts, and are all 64bit (and assumed to never > overflow). Rather than duplicate the code that we already have for managing > timing when reading from counters that don't have interrupts (the functions > that are exposed by this patch), I've reused it. So note that all the software counters generate interrupts in their own measuring domain. The hrtimer ones measure time and generate time based interrupts, the event based ones generate 'interrupts' on their events. What you have here is a hw pmu without interrupt capability. That's fine, they don't get to generate interrupt. We have plenty of those already. But what you propose to do is add interrupt in another domain entirely. That's not fine. Don't do that. You also try and conceal this information; so you suck. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers
On 02/25/2014 02:20 AM, Peter Zijlstra wrote: On Tue, Feb 25, 2014 at 02:33:26PM +1100, Michael Ellerman wrote: On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote: Export the swevent hrtimer helpers currently only used in events/core.c to allow the addition of architecture specific sw-like pmus. Peter, Ingo, can we get your ACK on this please? How are they used? I saw some usage in patch 9 or so; but its not explained anywhere. All patches have non-existent Changelogs and the few comments that are there are pretty hardware specific. So please do tell; what do you need this for? From this patch's change log: Export the swevent hrtimer helpers currently only used in events/core.c to allow the addition of architecture specific sw-like pmus. The key part here is "architecture specific sw-like pmus", where the announcement explains why these pmus are sw-like: The counters supplied by these interfaces are continually counting and never need to be (and cannot be) disabled or enabled. They additionally do not generate any interrupts. This makes them in some regards similar to software counters, and as a result their implimentation shares some common code (which an initial patch exposes) with the sw counters. Essentially, these pmus just provide access to a big array of counters which don't generate interrupts, and are all 64bit (and assumed to never overflow). Rather than duplicate the code that we already have for managing timing when reading from counters that don't have interrupts (the functions that are exposed by this patch), I've reused it. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers
On Tue, Feb 25, 2014 at 02:33:26PM +1100, Michael Ellerman wrote: > On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote: > > Export the swevent hrtimer helpers currently only used in events/core.c > > to allow the addition of architecture specific sw-like pmus. > > Peter, Ingo, can we get your ACK on this please? How are they used? I saw some usage in patch 9 or so; but its not explained anywhere. All patches have non-existent Changelogs and the few comments that are there are pretty hardware specific. So please do tell; what do you need this for? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 02/11] perf core: export swevent hrtimer helpers
On Fri, 2014-14-02 at 22:02:06 UTC, Cody P Schafer wrote:
> Export the swevent hrtimer helpers currently only used in events/core.c
> to allow the addition of architecture specific sw-like pmus.
Peter, Ingo, can we get your ACK on this please?
cheers
> Signed-off-by: Cody P Schafer
> ---
> include/linux/perf_event.h | 5 -
> kernel/events/core.c | 8
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 2702e91..24378a9 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -559,7 +559,10 @@ extern void perf_pmu_migrate_context(struct pmu *pmu,
> int src_cpu, int dst_cpu);
> extern u64 perf_event_read_value(struct perf_event *event,
>u64 *enabled, u64 *running);
> -
> +extern void perf_swevent_init_hrtimer(struct perf_event *event);
> +extern void perf_swevent_start_hrtimer(struct perf_event *event);
> +extern void perf_swevent_cancel_hrtimer(struct perf_event *event);
> +extern int perf_swevent_event_idx(struct perf_event *event);
>
> struct perf_sample_data {
> u64 type;
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 56003c6..feb0347 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5816,7 +5816,7 @@ static int perf_swevent_init(struct perf_event *event)
> return 0;
> }
>
> -static int perf_swevent_event_idx(struct perf_event *event)
> +int perf_swevent_event_idx(struct perf_event *event)
> {
> return 0;
> }
> @@ -6045,7 +6045,7 @@ static enum hrtimer_restart perf_swevent_hrtimer(struct
> hrtimer *hrtimer)
> return ret;
> }
>
> -static void perf_swevent_start_hrtimer(struct perf_event *event)
> +void perf_swevent_start_hrtimer(struct perf_event *event)
> {
> struct hw_perf_event *hwc = &event->hw;
> s64 period;
> @@ -6067,7 +6067,7 @@ static void perf_swevent_start_hrtimer(struct
> perf_event *event)
> HRTIMER_MODE_REL_PINNED, 0);
> }
>
> -static void perf_swevent_cancel_hrtimer(struct perf_event *event)
> +void perf_swevent_cancel_hrtimer(struct perf_event *event)
> {
> struct hw_perf_event *hwc = &event->hw;
>
> @@ -6079,7 +6079,7 @@ static void perf_swevent_cancel_hrtimer(struct
> perf_event *event)
> }
> }
>
> -static void perf_swevent_init_hrtimer(struct perf_event *event)
> +void perf_swevent_init_hrtimer(struct perf_event *event)
> {
> struct hw_perf_event *hwc = &event->hw;
>
> --
> 1.8.5.4
>
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
