Subject: Re: [PATCH 0/3] jfs: logging neatening
To: Joe Perches <joe@perches.com>,
        JFS Discussion <jfs-discussion@lists.sourceforge.net>
References: <cover.1459340028.git.joe@perches.com> <56FBF747.60306@oracle.com>
 <1459354939.25110.123.camel@perches.com>
Cc: linux-kernel@vger.kernel.org
From: Dave Kleikamp <dave.kleikamp@oracle.com>
Message-ID: <56FBFE3E.7050505@oracle.com>
Date: Wed, 30 Mar 2016 11:26:38 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.1
MIME-Version: 1.0
In-Reply-To: <1459354939.25110.123.camel@perches.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2098
Lines: 54

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.