On 3 November 2017 at 04:08, Suzuki K Poulose Suzuki.Poulose@arm.com wrote:
On 02/11/17 20:26, Mathieu Poirier wrote:
On Thu, Oct 19, 2017 at 06:15:47PM +0100, Suzuki K Poulose wrote:
Since the ETR could be driven either by SYSFS or by perf, it becomes complicated how we deal with the buffers used for each of these modes. The ETR driver cannot simply free the current attached buffer without knowing the provider (i.e, sysfs vs perf).
To solve this issue, we provide:
- the driver-mode specific etr buffer to be retained in the drvdata
- the etr_buf for a session should be passed on when enabling the hardware, which will be stored in drvdata->etr_buf. This will be replaced (not free'd) as soon as the hardware is disabled, after necessary sync operation.
If I get you right the problem you're trying to solve is what to do with a sysFS buffer that hasn't been read (and freed) when a perf session is requested. In my opinion it should simply be freed. Indeed the user probably doesn't care much about that sysFS buffer, if it did the data would have been harvested.
Not only that. If we simply use the drvdata->etr_buf, we cannot track the mode which uses it. If we keep the etr_buf around, how do the new mode user decide how to free the existing one ? (e.g, the perf etr_buf could be associated with other perf data structures). This change would allow us to leave the handling of the etr_buf to its respective modes.
struct etr_buf has a 'mode' and an '*ops', how is that not sufficient? I'll try to finish reviewing your patches today, maybe I'll find the answer later on...
And whether to keep the sysfs etr_buf around is a separate decision from the above.
Cheers Suzuki