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

[Dave Kleikamp <dave.kleikamp@oracle.com>]

On 03/30/2016 11:22 AM, Joe Perches wrote:
> On Wed, 2016-03-30 at 10:56 -0500, Dave Kleikamp wrote:
>> On 03/30/2016 07:23 AM, Joe Perches wrote:
>>>
>>> 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.

Okay.

> Many of the jfs_info calls appear to be function
> tracing and perhaps could be eliminated altogether.

Yeah. They've been in there forever. Should probably have been stripped
out before the code was initially merged.