On Thu, 24 Mar 2022 at 18:17, Dave Chinner <david@xxxxxxxxxxxxx> wrote: > Hmmmm. This is still missing a description of what "semi-stable" > implies in terms of kernel/userspace ABI guarantees/constraints for > the XFS codebase and developers. I'll reword this in a followup patch. The printk index just captures and exposes these strings from the sources. Its design was chosen specifically because it places no new constraints on kernel developers. This functionality enables kernel developers to change format strings at will, and when captured by the printk index, enables end users to detect changes from release to release (and presumably trigger them to update any relevant regexes on their end). > However, change logs should not be part of the commit message - they > go either in the cover message or below the "---" line as we don't > record them in the git history when the code is merged. > > Most importantly, the patch is missing a Signed-off-by line. I've got these fixed up now in my format-patch configuration and use of notes. Sorry for the noise in this regard, I don't usually email patches around. > Now the kern_level comes from the high level macros, we don't need > these constructors any more. This just results in identical functions > that differ only by name. i.e. this constructor macro and the > functions it builds can be replaced with a single function This is a good suggestion for simplifying the existing functionality and making the logical change to switch in printk indexing much smaller. > And then if you split this patch into two - the first patch > reorganises the printf code, the second introduces the new printk > functionality (i.e xfs_printk_index_wrap() macro) - then we can > review and merge the cleanup patch independently of the fate of the > second patch that may introduce ABI contraints.... Seems reasonable. I'll follow up with a PATCH v2 incorporating these suggestions. -- jof
