lkml.org 
[lkml]   [2015]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC PATCH 14/20] coresight: etm-perf: implementing 'event_init()' API
    Date
    Mathieu Poirier <mathieu.poirier@linaro.org> writes:

    > +static void etm_event_destroy(struct perf_event *event)
    > +{
    > + /* switching off the source will also tear down the path */
    > + etm_event_power_sources(event->cpu, false);
    > +}
    > +
    > +static int etm_event_init(struct perf_event *event)
    > +{
    > + int ret;
    > +
    > + if (event->attr.type != etm_pmu.type)
    > + return -ENOENT;
    > +
    > + if (event->cpu >= nr_cpu_ids)
    > + return -EINVAL;
    > +
    > + /* only one session at a time */
    > + if (etm_event_source_enabled(event->cpu))
    > + return -EBUSY;

    Why is this the case? If you were to configure the event in pmu::add()
    and deconfigure it in pmu::del(), like you already do with the buffer
    part, you could handle as many sessions as you want.

    > +
    > + /*
    > + * Make sure CPUs don't disappear between the
    > + * power up sequence and configuration.
    > + */
    > + get_online_cpus();
    > + ret = etm_event_power_sources(event->cpu, true);
    > + if (ret)
    > + goto out;
    > +
    > + ret = etm_event_config_sources(event->cpu);

    This can be done in pmu::add(), if you can call directly into
    etm_configure_cpu() or etm_config_enable() so that there's no cross-cpu
    calling in between.

    > +
    > + event->destroy = etm_event_destroy;
    > +out:
    > + put_online_cpus();
    > + return ret;
    > +}
    > +
    > static int __init etm_perf_init(void)
    > {
    > etm_pmu.capabilities = PERF_PMU_CAP_EXCLUSIVE;
    > @@ -59,6 +234,7 @@ static int __init etm_perf_init(void)
    > etm_pmu.attr_groups = etm_pmu_attr_groups;
    > etm_pmu.task_ctx_nr = perf_sw_context;
    > etm_pmu.read = etm_event_read;
    > + etm_pmu.event_init = etm_event_init;
    >
    > return perf_pmu_register(&etm_pmu, CORESIGHT_ETM_PMU_NAME, -1);
    > }

    One general comment -- it would be slightly easier at least for me if
    all the pmu related bits were in one patch.

    Thanks,
    --
    Alex


    \
     
     \ /
      Last update: 2015-09-22 16:41    [W:3.339 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site