Re: [PATCH 0/3] jfs: logging neatening
From: Joe Perches
Date: Wed Mar 30 2016 - 12:23:10 EST
On Wed, 2016-03-30 at 10:56 -0500, Dave Kleikamp wrote:
> On 03/30/2016 07:23 AM, Joe Perches wrote:
> > This patchset fixes the uses of jfs_info, jfs_warn and jfs_err that
> > have terminating newlines and a couple other trivialities to make
> > the logging a bit more consistent.
> These patches look good. I'm pushing them out to the -next build.
> > There is a difference in use between jfs_error and the other
> > jfs_info, jfs_warn, and jfs_err logging macros. jfs_error is more
> > like the rest of the kernel and requires a newline as the last
> > character of the format.
> > The jfs_info, jfs_warn, and jfs_err macros add the terminating
> > newline to the format so the uses do not require them.
> I think there's an argument for both ways of doing it. I'm sure I had my
> reasons for automatically adding the newline back when I implemented
> those macros. (They probably should be inline functions, but that's
> another issue.)
Nah. It was me. I changed jfs_error awhile back to move the
newline to the uses.
Author: Joe Perches <joe@xxxxxxxxxxx>
Date: Tue Jun 4 16:39:15 2013 -0700
jfs: Update jfs_error
Use a more current logging style.
Add __printf format and argument verification.
Remove embedded function names from formats.
Add %pf, __builtin_return_address(0) to jfs_error.
Add newlines to formats for kernel style consistency.
(One format already had an erroneous newline)
Coalesce formats and align arguments.
Object size reduced ~1KiB.
$ size fs/jfs/built-in.o*
text data bss dec hex filename
201891 35488 63936 301315 49903 fs/jfs/built-in.o.new
202821 35488 64192 302501 49da5 fs/jfs/built-in.o.old
Using inline functions would actually be more code as
you'd have to handle the log level and newline via
a vprintk of some type. At least the test could be
consolidated into the inline though.
Many of the jfs_info calls appear to be function
tracing and perhaps could be eliminated altogether.
> It might be better if the jfs_info, jfs_warn, and jfs_err macros
> > were changed to require a newline termination and the uses changed
> > to include the newline, but that's a larger change.
> Yeah, these patches are the obvious improvement, without changing
> anything from a design standpoint.