Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1367170 > unrolled thread
| Started by | Joe Perches <joe@perches.com> |
|---|---|
| First post | 2016-03-30 14:30 +0200 |
| Last post | 2016-03-30 18:30 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] jfs: logging neatening Joe Perches <joe@perches.com> - 2016-03-30 14:30 +0200
Re: [PATCH 0/3] jfs: logging neatening Dave Kleikamp <dave.kleikamp@oracle.com> - 2016-03-30 18:00 +0200
Re: [PATCH 0/3] jfs: logging neatening Dave Kleikamp <dave.kleikamp@oracle.com> - 2016-03-30 18:30 +0200
Re: [PATCH 0/3] jfs: logging neatening Joe Perches <joe@perches.com> - 2016-03-30 18:30 +0200
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-03-30 14:30 +0200 |
| Subject | [PATCH 0/3] jfs: logging neatening |
| Message-ID | <rinq2-5iE-13@gated-at.bofh.it> |
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. 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. It is a habituated style for many people to add the terminating newline for many people and this difference in use between the various macro styles causes trivial defects in logging output. 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. Joe Perches (3): jfs: Remove terminating newlines from jfs_info, jfs_warn, jfs_err uses jfs: Remove unnecessary line continuations and terminating newlines jfs: Coalesce some formats fs/jfs/inode.c | 4 ++-- fs/jfs/jfs_discard.c | 6 ++---- fs/jfs/jfs_dtree.c | 10 ++++------ fs/jfs/jfs_imap.c | 3 +-- fs/jfs/jfs_inode.c | 2 +- fs/jfs/jfs_logmgr.c | 14 ++++++-------- fs/jfs/jfs_txnmgr.c | 21 +++++++++------------ fs/jfs/namei.c | 4 ++-- fs/jfs/super.c | 4 ++-- 9 files changed, 29 insertions(+), 39 deletions(-) -- 2.8.0.rc4.16.g56331f8
[toc] | [next] | [standalone]
| From | Dave Kleikamp <dave.kleikamp@oracle.com> |
|---|---|
| Date | 2016-03-30 18:00 +0200 |
| Message-ID | <riqHh-7AW-41@gated-at.bofh.it> |
| In reply to | #1367170 |
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.) > It is a habituated style for many people to add the terminating > newline for many people and this difference in use between the > various macro styles causes trivial defects in logging output. > > 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. > > Joe Perches (3): > jfs: Remove terminating newlines from jfs_info, jfs_warn, jfs_err uses > jfs: Remove unnecessary line continuations and terminating newlines > jfs: Coalesce some formats > > fs/jfs/inode.c | 4 ++-- > fs/jfs/jfs_discard.c | 6 ++---- > fs/jfs/jfs_dtree.c | 10 ++++------ > fs/jfs/jfs_imap.c | 3 +-- > fs/jfs/jfs_inode.c | 2 +- > fs/jfs/jfs_logmgr.c | 14 ++++++-------- > fs/jfs/jfs_txnmgr.c | 21 +++++++++------------ > fs/jfs/namei.c | 4 ++-- > fs/jfs/super.c | 4 ++-- > 9 files changed, 29 insertions(+), 39 deletions(-) >
[toc] | [prev] | [next] | [standalone]
| From | Dave Kleikamp <dave.kleikamp@oracle.com> |
|---|---|
| Date | 2016-03-30 18:30 +0200 |
| Message-ID | <riram-82U-77@gated-at.bofh.it> |
| In reply to | #1367326 |
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.
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-03-30 18:30 +0200 |
| Message-ID | <riram-82U-75@gated-at.bofh.it> |
| In reply to | #1367326 |
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.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web