lkml.org 
[lkml]   [2015]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH V2 20/30] coresight: etb10: implementing buffer set/reset() API
    Date
    Mathieu Poirier <mathieu.poirier@linaro.org> writes:

    > Implementing perf related APIs to activate and terminate
    > a trace session. More specifically dealing with the sink
    > buffer's internal mechanic along with perf's API to start
    > and stop interactions with the ring buffers.

    A matter of preference, but I'd say that it would be easier to review
    this part if you merged all the buffer related patches together.

    > +static void etb_reset_buffer(struct coresight_device *csdev,
    > + struct perf_output_handle *handle,
    > + void *sink_config)
    > +{
    > + struct cs_buffers *buf = sink_config;
    > + struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
    > +
    > + if (buf) {
    > + /*
    > + * In snapshot mode ->data_size holds the new address of the
    > + * ring buffer's head. The size itself is the whole address
    > + * range since we want the latest information.
    > + */
    > + if (buf->snapshot)
    > + handle->head = local_xchg(&buf->data_size,
    > + buf->nr_pages << PAGE_SHIFT);

    Does it make sense to do this in etb_update_buffer() instead?

    > + perf_aux_output_end(handle, local_xchg(&buf->data_size, 0),
    > + local_xchg(&buf->lost, 0));

    The corresponding perf_aux_output_begin() is done in etm_event_add(),
    I'd suggest that you do this in etm_event_del(),
    unconditionally. Otherwise you're risking ending up with a refcount leak
    and all sorts of horror.

    Regards,
    --
    Alex


    \
     
     \ /
      Last update: 2015-10-20 12:21    [W:4.100 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site