Re: [PATCH v2 09/11] coresight: etm4x: docs: Update ABI doc for sysfs features added.

Mathieu Poirier
2019-09-04T16:06:13Z

On Tue, 3 Sep 2019 at 23:48, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Sep 03, 2019 at 04:51:40PM -0600, Mathieu Poirier wrote:
> > On Tue, 3 Sep 2019 at 13:59, Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Aug 29, 2019 at 10:33:19PM +0100, Mike Leach wrote:
> > > > Update document to include the new sysfs features added during this
> > > > patchset.
> > > >
> > > > Updated to reflect the new sysfs component nameing schema.
> > > >
> > > > Signed-off-by: Mike Leach <mike.leach@linaro.org>
> > > > ---
> > > > .../testing/sysfs-bus-coresight-devices-etm4x | 183 +++++++++++-------
> > > > 1 file changed, 115 insertions(+), 68 deletions(-)
> > > >
> > > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > index 36258bc1b473..112c50ae9986 100644
> > > > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
> > > > @@ -1,4 +1,4 @@
> > > > -What: /sys/bus/coresight/devices/<memory_map>.etm/enable_source
> > > > +What: /sys/bus/coresight/devices/etm<N>/enable_source
> > >
> > > You are renaming sysfs directories that have been around since:
> > >
> > > > Date: April 2015
> > >
> > > ???
> > >
> > > Really?
> > >
> > > That's brave.
> >
> >
> > When I worked on the coresight sysfs ABI a while back I specifically
> > added it at the "testing" level as I was well aware that things could
> > change in the future. According to the guidelines in the
> > documentation userspace can rely on it which was accurate since the
> > interface didn't change for 4 years. But the guidelines also mention
> > that changes can occur before the interfaces are move to stables, and
> > that programs are encouraged to manifest their interest by adding
> > their name to the "users" field.
> >
> > The interface was changed in 5.2 to support coresight from ACPI and
> > make things easier to understand for users. It is a lot more
> > intuitive to associate an ETM tracer with the CPU it belongs to by
> > referring to the CPU number than the memory mapped address. Given the
> > "testing" status of the interface and the absence of registered users
> > I decided to move forward with the change. If "testing" is too strict
> > for that I suggest to add an "experimental" category where it would be
> > more acceptable to change things as subsystems mature.
>
> "testing" is not really "testing" if you have userspace tools/programs
> assuming the location and contents of specific files in sysfs.
>
> You can change things in sysfs by creating new files, but to do
> wholesale renaming like you did here can be very dangerous as you might
> be breaking things.

Yes, something I have definitely considered.

> Usually new files are created, not existing ones
> moved.

In this case it would have meant a new symbolic link for every
coresight device, so twice a many entries under
$(SYS)/bus/coresight/device/. That would have been a lot of clutter
and an increasing source of problems as the number of CPU and sinks
increases. To me, and given the permissive definition of "testing"
found in the documentation, a clean break was a better option.

>
> What tools use these today? What is going to break?

Other than local shell scripts I am not aware of any tools using these
today. I am certainly open to discuss a better alternative but right
now, I just don't see one.

>
> thanks,
> greg k-h