Re: [PATCH 2/3] cxl: Introduce module parameter 'enable_psltrace'

[Frederic Barrat <fbarrat@linux.vnet.ibm.com>]

Le 11/02/2018 à 18:10, Vaibhav Jain a écrit :
> Thanks for reviewing the patch Christophe,
> 
> christophe lombard <clombard@linux.vnet.ibm.com> writes:
>>> +bool cxl_enable_psltrace = true;
>>> +module_param_named(enable_psltrace, cxl_enable_psltrace, bool, 0600);
>>> +MODULE_PARM_DESC(enable_psltrace, "Set PSL traces on probe. default: on");
>>> +
>> I am not too agree to add a new parameter. This can cause doubts.
>> PSL team has confirmed that enabling traces has no impact.
>> Do you see any reason to disable the traces ?
> 
> Traces on PSL follow a 'set and fetch' model. So once the trace buffer for
> a specific array is full it will stop and switch to 'FIN' state and at
> that point we need to fetch the trace-data and reinit the array to
> re-arm it.

If the PSL trace arrays don't wrap, is there anything to gain by 
enabling tracing by default instead of letting the developer handle it 
through sysfs? I was under the (now wrong) impression that the PSL would 
wrap.
I'm not a big fan of the module parameter. It seems we're giving a 
second way of activating traces on top of sysfs, more cumbersome and 
limited.

   Fred

> There might be some circumstances where this model may lead to confusion
> specifically when AFU developers assume that the trace arrays are
> already armed and dont re-arm it causing miss of trace data.
> 
> So this module param is a compromise to keep the old behaviour of traces
> array intact where in the arming/disarming of the trace arrays is
> controlled completely by userspace tooling and not by cxl.
>