Re: [PATCH v3 2/2] Add XFS messages to printk index

[Chris Down <chris@chrisdown.name>]

Hi Dave,

Dave Chinner writes:
>I ask, because user/kernel ABIs are usually fixed and we are not
>allowed to change them in a way that might break userspace. What
>happens when one of these format messages gets moved? What if the
>file, function and line of code all change, but the format string
>stays the same? What about duplicate format strings in different
>files/functions?

printk indexing is exposed in debugfs, and we have a long standing 
understanding that debugfs APIs are not stable, and there are no ABI 
guarantees. The statement in the initial patch that the API is "semi-stable" is 
simply wrong.

>Exactly how is this supposed to be used by userspace? Given that you
>are exposing both the file and the line of the file that the format
>string belongs to, does this mean we can no longer actually move
>this format string to any other location in the source code?

>
>IOWs, I cannot find anything that documents the implications of
>directly exposing the *raw source code* to userspace though a sysfs
>file on either developers or userspace applications.  Exposing
>anything through a sysfs file usually comes with constraints and
>guarantees and just because it is in /sys/kernel/debug means we can
>waive ABI constraints: I'll refer you to the canonical example of
>tracepoints vs powertop.
>
>With tracepoints in mind, XFS has an explicit policy that
>tracepoints do not form part of the user ABI because they expose the
>internal implementation directly to userspace. Hence if you use XFS
>tracepoints for any purpose, you get to keep all the broken bits
>when we change/add/remove tracepoints as part of our normal
>development.
>
>However, for information we explicitly expose via files in proc and
>sysfs (and via ioctls, for that matter), we have to provide explicit
>ABI guarantees, and that means we cannot remove or change the format
>or content of those files in ways that would cause userspace parsers
>and applications to break. If we are removing a proc/sysfs file, we
>have an explicit deprecation process that takes years to run so that
>userspace has time to notice that removal will be occurring and be
>updated not to depend on it by the time we remove it.

debugfs has no stability guarantees whatsoever and exists outside of the 
userspace ABI guarantees.[0]

Even if there was some guarantee (which there isn't), the guarantee would be on 
the format of the file, not the data contained within. The point of printk 
indexing is to indicate when things change, not preclude that change. Just as a 
map appearing, changing, or disappearing in /proc/pid/smaps isn't an ABI break, 
this wouldn't be either. It would just be a change in the backing data. For 
example, if a file or line changes, printk indexing helps indicate to a 
userspace tool that the printk may have gone away, or changed location.

Anyone using printk indexing has to accept that both the format and the file 
contents are not stable between kernel releases, and must accommodate for that 
during development. That's the main reason that this was put in debugfs rather 
than (for example) /proc or /sys.

>I see no statement anywhere about what this printk index ABI
>requires in terms of code stablility, format string maintenance and
>modification, etc. I've seen it referred to as "semi-stable" but
>there is no clear, easily accessible definition as to what that
>means for either kernel developers or userspace app developers that
>might want to use this information. There's zero information
>available about how userspace will use this information, too, so at
>this point I can't even guess what the policy for this new ABI
>actually is.

I don't know why Jonathan referred to it as "semi-stable". It is simply not so. 
printk indexing is a completely unstable debugfs API with no guarantees about 
either format or contents to userspace at all over kernel releases.

>If this was discussed and a policy was created, then great. But it
>*hasn't been documented* for the rest of the world to be able to
>read and understand so they know how to deal safely with the
>information this ABI now provides. So, can you please explain what
>the rules are, and then please write some documentation for the
>kernel admin guide defining the user ABI for application writers and
>what guarantees the kernel provides them with about the contents of
>this ABI.

The policy, as with all debugfs APIs by default, is that it's completely 
unstable and there are no API stability guarantees whatsoever. That's why 
there's no extensive documentation for users: because this is a feature for 
kernel developers.

0: https://lwn.net/Articles/309298/

Thanks,

Chris