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

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Sep 04, 2019 at 10:05:51AM -0600, Mathieu Poirier wrote:
> 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.

Well, "testing" doesn't really matter in the end, if a tool/user relies
on it, we have to keep it working properly.

> > 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.

Be aware that you might have to change this back if there is any
objections.

thanks,

greg k-h