* [patch 1/1] fs/proc: clean up printks
@ 2013-01-31 22:20 akpm
2013-02-03 20:59 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: akpm @ 2013-01-31 22:20 UTC (permalink / raw)
To: linux-fsdevel; +Cc: akpm, joe
From: Andrew Morton <akpm@linux-foundation.org>
Subject: fs/proc: clean up printks
- use pr_foo() throughout
- remove a couple of duplicated KERN_WARNINGs, via WARN(KERN_WARNING "...")
- nuke a few warnings which I've never seen happen, ever.
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/proc/base.c | 3 ++-
fs/proc/generic.c | 23 +++++++----------------
fs/proc/inode.c | 5 +++--
fs/proc/kcore.c | 3 ++-
fs/proc/proc_devtree.c | 13 +++++++------
fs/proc/proc_sysctl.c | 19 ++++++++++---------
fs/proc/vmcore.c | 15 ++++++---------
7 files changed, 37 insertions(+), 44 deletions(-)
diff -puN fs/proc/base.c~fs-proc-clean-up-printks fs/proc/base.c
--- a/fs/proc/base.c~fs-proc-clean-up-printks
+++ a/fs/proc/base.c
@@ -73,6 +73,7 @@
#include <linux/security.h>
#include <linux/ptrace.h>
#include <linux/tracehook.h>
+#include <linux/printk.h>
#include <linux/cgroup.h>
#include <linux/cpuset.h>
#include <linux/audit.h>
@@ -952,7 +953,7 @@ static ssize_t oom_adj_write(struct file
* /proc/pid/oom_adj is provided for legacy purposes, ask users to use
* /proc/pid/oom_score_adj instead.
*/
- printk_once(KERN_WARNING "%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
+ pr_warn_once("%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
current->comm, task_pid_nr(current), task_pid_nr(task),
task_pid_nr(task));
diff -puN fs/proc/generic.c~fs-proc-clean-up-printks fs/proc/generic.c
--- a/fs/proc/generic.c~fs-proc-clean-up-printks
+++ a/fs/proc/generic.c
@@ -15,6 +15,7 @@
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/printk.h>
#include <linux/mount.h>
#include <linux/init.h>
#include <linux/idr.h>
@@ -132,11 +133,8 @@ __proc_file_read(struct file *file, char
}
if (start == NULL) {
- if (n > PAGE_SIZE) {
- printk(KERN_ERR
- "proc_file_read: Apparent buffer overflow!\n");
+ if (n > PAGE_SIZE) /* Apparent buffer overflow */
n = PAGE_SIZE;
- }
n -= *ppos;
if (n <= 0)
break;
@@ -144,26 +142,19 @@ __proc_file_read(struct file *file, char
n = count;
start = page + *ppos;
} else if (start < page) {
- if (n > PAGE_SIZE) {
- printk(KERN_ERR
- "proc_file_read: Apparent buffer overflow!\n");
+ if (n > PAGE_SIZE) /* Apparent buffer overflow */
n = PAGE_SIZE;
- }
if (n > count) {
/*
* Don't reduce n because doing so might
* cut off part of a data block.
*/
- printk(KERN_WARNING
- "proc_file_read: Read count exceeded\n");
+ pr_warn("proc_file_read: count exceeded\n");
}
} else /* start >= page */ {
unsigned long startoff = (unsigned long)(start - page);
- if (n > (PAGE_SIZE - startoff)) {
- printk(KERN_ERR
- "proc_file_read: Apparent buffer overflow!\n");
+ if (n > (PAGE_SIZE - startoff)) /* buffer overflow? */
n = PAGE_SIZE - startoff;
- }
if (n > count)
n = count;
}
@@ -576,7 +567,7 @@ static int proc_register(struct proc_dir
for (tmp = dir->subdir; tmp; tmp = tmp->next)
if (strcmp(tmp->name, dp->name) == 0) {
- WARN(1, KERN_WARNING "proc_dir_entry '%s/%s' already registered\n",
+ WARN(1, "proc_dir_entry '%s/%s' already registered\n",
dir->name, dp->name);
break;
}
@@ -837,7 +828,7 @@ void remove_proc_entry(const char *name,
if (S_ISDIR(de->mode))
parent->nlink--;
de->nlink = 0;
- WARN(de->subdir, KERN_WARNING "%s: removing non-empty directory "
+ WARN(de->subdir, "%s: removing non-empty directory "
"'%s/%s', leaking at least '%s'\n", __func__,
de->parent->name, de->name, de->subdir->name);
pde_put(de);
diff -puN fs/proc/inode.c~fs-proc-clean-up-printks fs/proc/inode.c
--- a/fs/proc/inode.c~fs-proc-clean-up-printks
+++ a/fs/proc/inode.c
@@ -13,6 +13,7 @@
#include <linux/stat.h>
#include <linux/completion.h>
#include <linux/poll.h>
+#include <linux/printk.h>
#include <linux/file.h>
#include <linux/limits.h>
#include <linux/init.h>
@@ -498,14 +499,14 @@ int proc_fill_super(struct super_block *
pde_get(&proc_root);
root_inode = proc_get_inode(s, &proc_root);
if (!root_inode) {
- printk(KERN_ERR "proc_fill_super: get root inode failed\n");
+ pr_warn("proc_fill_super: get root inode failed\n");
pde_put(&proc_root);
return -ENOMEM;
}
s->s_root = d_make_root(root_inode);
if (!s->s_root) {
- printk(KERN_ERR "proc_fill_super: allocate dentry failed\n");
+ pr_err("proc_fill_super: allocate dentry failed\n");
return -ENOMEM;
}
diff -puN fs/proc/kcore.c~fs-proc-clean-up-printks fs/proc/kcore.c
--- a/fs/proc/kcore.c~fs-proc-clean-up-printks
+++ a/fs/proc/kcore.c
@@ -17,6 +17,7 @@
#include <linux/elfcore.h>
#include <linux/vmalloc.h>
#include <linux/highmem.h>
+#include <linux/printk.h>
#include <linux/bootmem.h>
#include <linux/init.h>
#include <linux/slab.h>
@@ -619,7 +620,7 @@ static int __init proc_kcore_init(void)
proc_root_kcore = proc_create("kcore", S_IRUSR, NULL,
&proc_kcore_operations);
if (!proc_root_kcore) {
- printk(KERN_ERR "couldn't create /proc/kcore\n");
+ pr_err("couldn't create /proc/kcore\n");
return 0; /* Always returns 0. */
}
/* Store text area if it's special */
diff -puN fs/proc/proc_devtree.c~fs-proc-clean-up-printks fs/proc/proc_devtree.c
--- a/fs/proc/proc_devtree.c~fs-proc-clean-up-printks
+++ a/fs/proc/proc_devtree.c
@@ -8,6 +8,7 @@
#include <linux/time.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/printk.h>
#include <linux/stat.h>
#include <linux/string.h>
#include <linux/of.h>
@@ -110,8 +111,8 @@ void proc_device_tree_update_prop(struct
if (ent->data == oldprop)
break;
if (ent == NULL) {
- printk(KERN_WARNING "device-tree: property \"%s\" "
- " does not exist\n", oldprop->name);
+ pr_warn("device-tree: property \"%s\" does not exist\n",
+ oldprop->name);
} else {
ent->data = newprop;
ent->size = newprop->length;
@@ -153,8 +154,8 @@ static const char *fixup_name(struct dev
realloc:
fixed_name = kmalloc(fixup_len, GFP_KERNEL);
if (fixed_name == NULL) {
- printk(KERN_ERR "device-tree: Out of memory trying to fixup "
- "name \"%s\"\n", name);
+ pr_err("device-tree: Out of memory trying to fixup "
+ "name \"%s\"\n", name);
return name;
}
@@ -175,8 +176,8 @@ retry:
goto retry;
}
- printk(KERN_WARNING "device-tree: Duplicate name in %s, "
- "renamed to \"%s\"\n", np->full_name, fixed_name);
+ pr_warn("device-tree: Duplicate name in %s, renamed to \"%s\"\n",
+ np->full_name, fixed_name);
return fixed_name;
}
diff -puN fs/proc/proc_sysctl.c~fs-proc-clean-up-printks fs/proc/proc_sysctl.c
--- a/fs/proc/proc_sysctl.c~fs-proc-clean-up-printks
+++ a/fs/proc/proc_sysctl.c
@@ -5,6 +5,7 @@
#include <linux/sysctl.h>
#include <linux/poll.h>
#include <linux/proc_fs.h>
+#include <linux/printk.h>
#include <linux/security.h>
#include <linux/sched.h>
#include <linux/namei.h>
@@ -57,7 +58,7 @@ static void sysctl_print_dir(struct ctl_
{
if (dir->header.parent)
sysctl_print_dir(dir->header.parent);
- printk(KERN_CONT "%s/", dir->header.ctl_table[0].procname);
+ pr_cont("%s/", dir->header.ctl_table[0].procname);
}
static int namecmp(const char *name1, int len1, const char *name2, int len2)
@@ -134,9 +135,9 @@ static int insert_entry(struct ctl_table
else if (cmp > 0)
p = &(*p)->rb_right;
else {
- printk(KERN_ERR "sysctl duplicate entry: ");
+ pr_err("sysctl duplicate entry: ");
sysctl_print_dir(head->parent);
- printk(KERN_CONT "/%s\n", entry->procname);
+ pr_cont("/%s\n", entry->procname);
return -EEXIST;
}
}
@@ -927,9 +928,9 @@ found:
subdir->header.nreg++;
failed:
if (unlikely(IS_ERR(subdir))) {
- printk(KERN_ERR "sysctl could not get directory: ");
+ pr_err("sysctl could not get directory: ");
sysctl_print_dir(dir);
- printk(KERN_CONT "/%*.*s %ld\n",
+ pr_cont("/%*.*s %ld\n",
namelen, namelen, name, PTR_ERR(subdir));
}
drop_sysctl_table(&dir->header);
@@ -995,8 +996,8 @@ static int sysctl_err(const char *path,
vaf.fmt = fmt;
vaf.va = &args;
- printk(KERN_ERR "sysctl table check failed: %s/%s %pV\n",
- path, table->procname, &vaf);
+ pr_err("sysctl table check failed: %s/%s %pV\n",
+ path, table->procname, &vaf);
va_end(args);
return -EINVAL;
@@ -1510,9 +1511,9 @@ static void put_links(struct ctl_table_h
drop_sysctl_table(link_head);
}
else {
- printk(KERN_ERR "sysctl link missing during unregister: ");
+ pr_err("sysctl link missing during unregister: ");
sysctl_print_dir(parent);
- printk(KERN_CONT "/%s\n", name);
+ pr_cont("/%s\n", name);
}
}
}
diff -puN fs/proc/vmcore.c~fs-proc-clean-up-printks fs/proc/vmcore.c
--- a/fs/proc/vmcore.c~fs-proc-clean-up-printks
+++ a/fs/proc/vmcore.c
@@ -15,6 +15,7 @@
#include <linux/export.h>
#include <linux/slab.h>
#include <linux/highmem.h>
+#include <linux/printk.h>
#include <linux/bootmem.h>
#include <linux/init.h>
#include <linux/crash_dump.h>
@@ -553,8 +554,7 @@ static int __init parse_crash_elf64_head
ehdr.e_ehsize != sizeof(Elf64_Ehdr) ||
ehdr.e_phentsize != sizeof(Elf64_Phdr) ||
ehdr.e_phnum == 0) {
- printk(KERN_WARNING "Warning: Core image elf header is not"
- "sane\n");
+ pr_warning("Warning: Core image elf header is not sane\n");
return -EINVAL;
}
@@ -609,8 +609,7 @@ static int __init parse_crash_elf32_head
ehdr.e_ehsize != sizeof(Elf32_Ehdr) ||
ehdr.e_phentsize != sizeof(Elf32_Phdr) ||
ehdr.e_phnum == 0) {
- printk(KERN_WARNING "Warning: Core image elf header is not"
- "sane\n");
+ pr_warning("Warning: Core image elf header is not sane\n");
return -EINVAL;
}
@@ -653,8 +652,7 @@ static int __init parse_crash_elf_header
if (rc < 0)
return rc;
if (memcmp(e_ident, ELFMAG, SELFMAG) != 0) {
- printk(KERN_WARNING "Warning: Core image elf header"
- " not found\n");
+ pr_warning("Warning: Core image elf header not found\n");
return -EINVAL;
}
@@ -673,8 +671,7 @@ static int __init parse_crash_elf_header
/* Determine vmcore size. */
vmcore_size = get_vmcore_size_elf32(elfcorebuf);
} else {
- printk(KERN_WARNING "Warning: Core image elf header is not"
- " sane\n");
+ pr_warning("Warning: Core image elf header is not sane\n");
return -EINVAL;
}
return 0;
@@ -690,7 +687,7 @@ static int __init vmcore_init(void)
return rc;
rc = parse_crash_elf_headers();
if (rc) {
- printk(KERN_WARNING "Kdump: vmcore not initialized\n");
+ pr_warning("Kdump: vmcore not initialized\n");
return rc;
}
_
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/1] fs/proc: clean up printks
2013-01-31 22:20 [patch 1/1] fs/proc: clean up printks akpm
@ 2013-02-03 20:59 ` Joe Perches
2013-02-07 23:49 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2013-02-03 20:59 UTC (permalink / raw)
To: akpm; +Cc: linux-fsdevel
On Thu, 2013-01-31 at 14:20 -0800, akpm@linux-foundation.org wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: fs/proc: clean up printks
>
> - use pr_foo() throughout
>
> - remove a couple of duplicated KERN_WARNINGs, via WARN(KERN_WARNING "...")
>
> - nuke a few warnings which I've never seen happen, ever.
Ignorable trivial comments only:
o Use pr_warn instead of pr_warning
o Consider using "%s: ...", __func__, ...
instead of "function_name: ...", ...
o As printk.h is included by kernel.h, and that
is unlikely to be changed, it's probably not
useful/necessary to add #include <linux/printk.h>
anywhere
o Consider using #define pr_fmt(fmt) where useful
o Consider coalescing formats
o Consider realigning arguments after name changes
> diff -puN fs/proc/base.c~fs-proc-clean-up-printks fs/proc/base.c
> @@ -952,7 +953,7 @@ static ssize_t oom_adj_write(struct file
[]
> - printk_once(KERN_WARNING "%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
> + pr_warn_once("%s (%d): /proc/%d/oom_adj is deprecated, please use /proc/%d/oom_score_adj instead.\n",
> current->comm, task_pid_nr(current), task_pid_nr(task),
> task_pid_nr(task));
Align args
> diff -puN fs/proc/generic.c~fs-proc-clean-up-printks fs/proc/generic.c
[]
> - printk(KERN_WARNING
> - "proc_file_read: Read count exceeded\n");
> + pr_warn("proc_file_read: count exceeded\n");
"%s: ...", __func__,
> @@ -837,7 +828,7 @@ void remove_proc_entry(const char *name,
[]
> - WARN(de->subdir, KERN_WARNING "%s: removing non-empty directory "
> + WARN(de->subdir, "%s: removing non-empty directory "
> "'%s/%s', leaking at least '%s'\n", __func__,
coalesce, align
> diff -puN fs/proc/inode.c~fs-proc-clean-up-printks fs/proc/inode.c
[]
> @@ -498,14 +499,14 @@ int proc_fill_super(struct super_block *
> - printk(KERN_ERR "proc_fill_super: get root inode failed\n");
> + pr_warn("proc_fill_super: get root inode failed\n");
"%s: ...", __func__,
> - printk(KERN_ERR "proc_fill_super: allocate dentry failed\n");
> + pr_err("proc_fill_super: allocate dentry failed\n");
etc...
> diff -puN fs/proc/proc_devtree.c~fs-proc-clean-up-printks fs/proc/proc_devtree.c
> --- a/fs/proc/proc_devtree.c~fs-proc-clean-up-printks
> +++ a/fs/proc/proc_devtree.c
> @@ -8,6 +8,7 @@
pr_fmt?
> diff -puN fs/proc/proc_sysctl.c~fs-proc-clean-up-printks fs/proc/proc_sysctl.c
[]
> @@ -995,8 +996,8 @@ static int sysctl_err(const char *path,
[]
> - printk(KERN_ERR "sysctl table check failed: %s/%s %pV\n",
> - path, table->procname, &vaf);
> + pr_err("sysctl table check failed: %s/%s %pV\n",
> + path, table->procname, &vaf);
Align args?
> diff -puN fs/proc/vmcore.c~fs-proc-clean-up-printks fs/proc/vmcore.c
[]
> @@ -553,8 +554,7 @@ static int __init parse_crash_elf64_head
[]
> - printk(KERN_WARNING "Warning: Core image elf header is not"
> - "sane\n");
> + pr_warning("Warning: Core image elf header is not sane\n");
pr_warn (many times)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch 1/1] fs/proc: clean up printks
2013-02-03 20:59 ` Joe Perches
@ 2013-02-07 23:49 ` Andrew Morton
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2013-02-07 23:49 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-fsdevel
On Sun, 03 Feb 2013 12:59:45 -0800
Joe Perches <joe@perches.com> wrote:
> On Thu, 2013-01-31 at 14:20 -0800, akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Subject: fs/proc: clean up printks
> >
> > - use pr_foo() throughout
> >
> > - remove a couple of duplicated KERN_WARNINGs, via WARN(KERN_WARNING "...")
> >
> > - nuke a few warnings which I've never seen happen, ever.
>
> Ignorable trivial comments only:
>
> o Use pr_warn instead of pr_warning
Yeah, I did that in the -fix patch.
> o Consider using "%s: ...", __func__, ...
> instead of "function_name: ...", ...
Too lazy ;)
> o As printk.h is included by kernel.h, and that
> is unlikely to be changed, it's probably not
> useful/necessary to add #include <linux/printk.h>
> anywhere
Too anal.
Failing to explicitly include the stuff you need often causes breakage
when header files are cleaned up, but we're unlikelly to weed printk.h
out of kernel.h.
> o Consider using #define pr_fmt(fmt) where useful
hm, where and how?
> o Consider coalescing formats
out of scope (ie: too lazy)
> o Consider realigning arguments after name changes
Fixed a couple.
--- a/fs/proc/generic.c~fs-proc-clean-up-printks-fix-fix
+++ a/fs/proc/generic.c
@@ -829,8 +829,8 @@ void remove_proc_entry(const char *name,
parent->nlink--;
de->nlink = 0;
WARN(de->subdir, "%s: removing non-empty directory "
- "'%s/%s', leaking at least '%s'\n", __func__,
- de->parent->name, de->name, de->subdir->name);
+ "'%s/%s', leaking at least '%s'\n", __func__,
+ de->parent->name, de->name, de->subdir->name);
pde_put(de);
}
EXPORT_SYMBOL(remove_proc_entry);
--- a/fs/proc/proc_sysctl.c~fs-proc-clean-up-printks-fix-fix
+++ a/fs/proc/proc_sysctl.c
@@ -997,7 +997,7 @@ static int sysctl_err(const char *path,
vaf.va = &args;
pr_err("sysctl table check failed: %s/%s %pV\n",
- path, table->procname, &vaf);
+ path, table->procname, &vaf);
va_end(args);
return -EINVAL;
_
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-02-07 23:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-31 22:20 [patch 1/1] fs/proc: clean up printks akpm
2013-02-03 20:59 ` Joe Perches
2013-02-07 23:49 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).