Re: [PATCH 0/3] jfs: logging neatening

[Joe Perches <joe@perches.com>]

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.

commit eb8630d7d2fd13589e6a7a3ae2fe1f75f867fbed
Author: Joe Perches <joe@perches.com>
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.