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:0.141 / U:1.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site