On Thu, 17 Mar 2022 at 16:16, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > That's a nasty mess. To begin with, we most definitely do not want > to have to define log level translations in multiple places so this > needs to be reworked so the front end macros define everything and > pass things down to the lower level functions. I can follow up with a PATCH v1 to do this kernel level deduplication. I focused my initial attempt at changing as little as possible. > And, anyway, why can't you just drop printk_index_subsys_emit() into > the define_xfs_printk_level() macro? The kern_level, the fmt string > and the varargs are all available there... The short answer is that the format strings need to be known at compile time, as this printk index is added into a section of the resultant ELF binary. > Anyway, there's more important high level stuff that needs > explaining first. > > This is competely undocumented functionality and it's the first I've > ever heard about it. There's nothing I can easily find to learn how > this information being exposed to userspace is supposed to be used. > The commit message is pretty much information free, but this is a > new userspace ABI. What ABI constraints are we now subject to by > exporting XFS log message formats to userspace places? > > i.e. Where's the documentation defining the contract this new > userspace ABI forms between the kernel log messages and userspace? > How do users know what we guarantee won't or will break, and how do > we kernel developers know what we're allowed to do once these very > specific internal subsystem implementation details are exposed to > userspace? > > Hell, how did this stuff even get merged without any supporting > documentation? I can't really comment as to the commit history of the printk indexing, though I think it's a fair criticism that there doesn't currently seem to be any in-tree Documentation file about the functionality yet. The best references I could point to are commit 337015573718b161891a3473d25f59273f2e626b and this LWN article: https://lwn.net/Articles/857148/ Your concern about ABI commitments is totally valid, and is (to me) rather ironic in this context. This printk indexing effort was started so that end user-operators can more reliably capture critical event data *without* asking developers to commit to anything. The hope here is that developers can change format strings and parameters at will, and that from release-to-release end user-operators can compare the printk index to see if there are any changes in printk index entries that they care about for their environment. It is my genuine hope that by merging a change like what I'm proposing here, that end users can more reliably detect and react to XFS events without XFS developers needing to know about this or commit to anything. The only reason this change is needed is because XFS wraps printk() for its own formatting.
