[PATCH V3 13/18] coresight: tmc: make sysFS and Perf mode mutually exclusive
Mathieu Poirier
mathieu.poirier at linaro.org
Mon Apr 25 08:18:04 PDT 2016
On 25 April 2016 at 09:11, Suzuki K Poulose <Suzuki.Poulose at arm.com> wrote:
> On 25/04/16 16:05, Mathieu Poirier wrote:
>>
>> On 25 April 2016 at 08:52, Suzuki K Poulose <Suzuki.Poulose at arm.com>
>> wrote:
>>>
>>> On 25/04/16 15:48, Mathieu Poirier wrote:
>>>>
>>>>
>>>> On 25 April 2016 at 08:32, Suzuki K Poulose <Suzuki.Poulose at arm.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 22/04/16 18:14, Mathieu Poirier wrote:
>>>
>>>
>>>
>>>>>> + spin_lock_irqsave(&drvdata->spinlock, flags);
>>>>>> + if (drvdata->reading) {
>>>>>> + ret = -EINVAL;
>>>>>> + goto out;
>>>>>> + }
>>>>>> +
>>>>>> + val = local_xchg(&drvdata->mode, mode);
>>>>>> + /*
>>>>>> + * In Perf mode there can be only one writer per sink. There
>>>>>> + * is also no need to continue if the ETR is already operated
>>>>>> + * from sysFS.
>>>>>> + */
>>>>>> + if (val != CS_MODE_DISABLED) {
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Could val be CS_MODE_PERF ? In other words, should we be checking :
>>>>> if (val == CS_MODE_SYSFS) instead ?
>>>>
>>>>
>>>>
>>>> If we check for CS_MODE_SYSFS we also have to check for CS_MODE_PERF,
>>>> which is two checks rather than a single one with the current
>>>> solution.
>>>
>>>
>>>
>>> I am confused now. The comment says, we want to check for sysfs mode and
>>> don't continue in that case. So, we shouldn't be worried about PERF mode.
>>
>>
>> You are correct about the sysFS part, but the first sentence of the
>> comment also mention that in perf mode there can only be one writer
>> per sink. Otherwise ring buffers for one session would end up with
>> traces from other ongoing sessions, and that is not taking into
>> account the buffer management nightmares it would cause.
>
>
> OK, in either case, val == CS_MODE_SYSFS is much better check there, to
> what we want to do
If we check for SYSFS we also need to check for PERF. Otherwise
nothing prevents another session from using the sink buffer, which is
not supported.
>
> Suzuki
More information about the linux-arm-kernel
mailing list