Message-ID: <1459354939.25110.123.camel@perches.com>
Subject: Re: [PATCH 0/3] jfs: logging neatening
From: Joe Perches <joe@perches.com>
To: Dave Kleikamp <dave.kleikamp@oracle.com>,
        JFS Discussion <jfs-discussion@lists.sourceforge.net>
Cc: linux-kernel@vger.kernel.org
Date: Wed, 30 Mar 2016 09:22:19 -0700
In-Reply-To: <56FBF747.60306@oracle.com>
References: <cover.1459340028.git.joe@perches.com>
	 <56FBF747.60306@oracle.com>
Content-Type: text/plain; charset="ISO-8859-1"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2421
Lines: 62

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.