Message ID | 20220525124816.86915-1-yuzhe@nfschina.com |
---|---|
State | Rejected |
Headers | show |
Series | ext4: add KERN_<LEVEL> for printk() | expand |
From: Yu Zhe > Sent: 25 May 2022 13:48 > > printk() should include KERN_<LEVEL> facility level, add them. They should also have an "ext3: " prefix. Isn't pr_warn() the function you are really looking for? David > > Signed-off-by: Yu Zhe <yuzhe@nfschina.com> > --- > fs/ext4/inode.c | 2 +- > fs/ext4/namei.c | 14 +++++++------- > fs/ext4/super.c | 4 ++-- > 3 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 3dce7d058985..6d6899191779 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -462,7 +462,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle, > if (es_map->m_lblk != map->m_lblk || > es_map->m_flags != map->m_flags || > es_map->m_pblk != map->m_pblk) { > - printk("ES cache assertion failed for inode: %lu " > + printk(KERN_WARNING "ES cache assertion failed for inode: %lu " > "es_cached ex [%d/%d/%llu/%x] != " > "found ex [%d/%d/%llu/%x] retval %d flags %x\n", > inode->i_ino, es_map->m_lblk, es_map->m_len, > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 47d0ca4c795b..89445661f71d 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -628,7 +628,7 @@ static struct stats dx_show_leaf(struct inode *dir, > char *base = (char *) de; > struct dx_hash_info h = *hinfo; > > - printk("names: "); > + printk(KERN_WARNING "names: "); > while ((char *) de < base + size) > { > if (de->inode) > @@ -648,7 +648,7 @@ static struct stats dx_show_leaf(struct inode *dir, > /* Directory is not encrypted */ > ext4fs_dirhash(dir, de->name, > de->name_len, &h); > - printk("%*.s:(U)%x.%u ", len, > + printk(KERN_WARNING "%*.s:(U)%x.%u ", len, > name, h.hash, > (unsigned) ((char *) de > - base)); > @@ -683,7 +683,7 @@ static struct stats dx_show_leaf(struct inode *dir, > else > ext4fs_dirhash(dir, de->name, > de->name_len, &h); > - printk("%*.s:(E)%x.%u ", len, name, > + printk(KERN_WARNING "%*.s:(E)%x.%u ", len, name, > h.hash, (unsigned) ((char *) de > - base)); > fscrypt_fname_free_buffer( > @@ -693,7 +693,7 @@ static struct stats dx_show_leaf(struct inode *dir, > int len = de->name_len; > char *name = de->name; > ext4fs_dirhash(dir, de->name, de->name_len, &h); > - printk("%*.s:%x.%u ", len, name, h.hash, > + printk(KERN_WARNING "%*.s:%x.%u ", len, name, h.hash, > (unsigned) ((char *) de - base)); > #endif > } > @@ -713,14 +713,14 @@ struct stats dx_show_entries(struct dx_hash_info *hinfo, struct inode *dir, > unsigned count = dx_get_count(entries), names = 0, space = 0, i; > unsigned bcount = 0; > struct buffer_head *bh; > - printk("%i indexed blocks...\n", count); > + printk(KERN_WARNING "%i indexed blocks...\n", count); > for (i = 0; i < count; i++, entries++) > { > ext4_lblk_t block = dx_get_block(entries); > ext4_lblk_t hash = i ? dx_get_hash(entries): 0; > u32 range = i < count - 1? (dx_get_hash(entries + 1) - hash): ~hash; > struct stats stats; > - printk("%s%3u:%03u hash %8x/%8x ",levels?"":" ", i, block, hash, range); > + printk(KERN_WARNING "%s%3u:%03u hash %8x/%8x ",levels?"":" ", i, block, hash, range); > bh = ext4_bread(NULL,dir, block, 0); > if (!bh || IS_ERR(bh)) > continue; > @@ -855,7 +855,7 @@ dx_probe(struct ext4_filename *fname, struct inode *dir, > goto fail; > } > > - dxtrace(printk("Look up %x", hash)); > + dxtrace(printk(KERN_WARNING "Look up %x", hash)); > level = 0; > blocks[0] = 0; > while (1) { > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 450c918d68fc..f1f0427b55a6 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -941,9 +941,9 @@ void __ext4_msg(struct super_block *sb, > vaf.fmt = fmt; > vaf.va = &args; > if (sb) > - printk("%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); > + printk(KERN_WARNING "%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); > else > - printk("%sEXT4-fs: %pV\n", prefix, &vaf); > + printk(KERN_WARNING "%sEXT4-fs: %pV\n", prefix, &vaf); > va_end(args); > } > > -- > 2.25.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, 2022-05-25 at 05:48 -0700, Yu Zhe wrote: > printk() should include KERN_<LEVEL> facility level, add them. NACK. checkpatch is just a guide. You have to inspect the code to determine if what checkpatch emits should be followed or ignored. > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c [] > @@ -628,7 +628,7 @@ static struct stats dx_show_leaf(struct inode *dir, > char *base = (char *) de; > struct dx_hash_info h = *hinfo; > > - printk("names: "); > + printk(KERN_WARNING "names: "); > while ((char *) de < base + size) > { > if (de->inode) > @@ -648,7 +648,7 @@ static struct stats dx_show_leaf(struct inode *dir, > /* Directory is not encrypted */ > ext4fs_dirhash(dir, de->name, > de->name_len, &h); > - printk("%*.s:(U)%x.%u ", len, > + printk(KERN_WARNING "%*.s:(U)%x.%u ", len, _might_ use KERN_CONT, > name, h.hash, > (unsigned) ((char *) de > - base)); > @@ -683,7 +683,7 @@ static struct stats dx_show_leaf(struct inode *dir, > else > ext4fs_dirhash(dir, de->name, > de->name_len, &h); > - printk("%*.s:(E)%x.%u ", len, name, > + printk(KERN_WARNING "%*.s:(E)%x.%u ", len, name, and here, etc... [] > diff --git a/fs/ext4/super.c b/fs/ext4/super.c [] > @@ -941,9 +941,9 @@ void __ext4_msg(struct super_block *sb, > vaf.fmt = fmt; > vaf.va = &args; > if (sb) > - printk("%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); > + printk(KERN_WARNING "%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); You broke the logging here. Did you check what prefix is and how it is used in ext4_msg ? > else > - printk("%sEXT4-fs: %pV\n", prefix, &vaf); > + printk(KERN_WARNING "%sEXT4-fs: %pV\n", prefix, &vaf); > va_end(args); > } >
On Wed, May 25, 2022 at 08:18:22AM -0700, Joe Perches wrote: > On Wed, 2022-05-25 at 05:48 -0700, Yu Zhe wrote: > > printk() should include KERN_<LEVEL> facility level, add them. > > NACK. > > checkpatch is just a guide. checkpatch-only patches are something which I will tend to just ignore. (Even if the changes are correct, which they are not in this case). In addition to all of the other reasons why such drive-by checkpatch patches are frowned upon, I would appreciate it if the code paths that are changed are actually *tested*, at which point you might have noticed that your changes werenn't doing the right thing. (BTW I'll also note that the code involved that is almost never enabled --- the developer would need to manually insert a "#define DX_DEBUG" at the beginning of the fs/ext4/namei.c source file for the code to be enabled. It's only intended to be used by developers who are doing surgery to the htree code.) - Ted
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 3dce7d058985..6d6899191779 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -462,7 +462,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle, if (es_map->m_lblk != map->m_lblk || es_map->m_flags != map->m_flags || es_map->m_pblk != map->m_pblk) { - printk("ES cache assertion failed for inode: %lu " + printk(KERN_WARNING "ES cache assertion failed for inode: %lu " "es_cached ex [%d/%d/%llu/%x] != " "found ex [%d/%d/%llu/%x] retval %d flags %x\n", inode->i_ino, es_map->m_lblk, es_map->m_len, diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 47d0ca4c795b..89445661f71d 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -628,7 +628,7 @@ static struct stats dx_show_leaf(struct inode *dir, char *base = (char *) de; struct dx_hash_info h = *hinfo; - printk("names: "); + printk(KERN_WARNING "names: "); while ((char *) de < base + size) { if (de->inode) @@ -648,7 +648,7 @@ static struct stats dx_show_leaf(struct inode *dir, /* Directory is not encrypted */ ext4fs_dirhash(dir, de->name, de->name_len, &h); - printk("%*.s:(U)%x.%u ", len, + printk(KERN_WARNING "%*.s:(U)%x.%u ", len, name, h.hash, (unsigned) ((char *) de - base)); @@ -683,7 +683,7 @@ static struct stats dx_show_leaf(struct inode *dir, else ext4fs_dirhash(dir, de->name, de->name_len, &h); - printk("%*.s:(E)%x.%u ", len, name, + printk(KERN_WARNING "%*.s:(E)%x.%u ", len, name, h.hash, (unsigned) ((char *) de - base)); fscrypt_fname_free_buffer( @@ -693,7 +693,7 @@ static struct stats dx_show_leaf(struct inode *dir, int len = de->name_len; char *name = de->name; ext4fs_dirhash(dir, de->name, de->name_len, &h); - printk("%*.s:%x.%u ", len, name, h.hash, + printk(KERN_WARNING "%*.s:%x.%u ", len, name, h.hash, (unsigned) ((char *) de - base)); #endif } @@ -713,14 +713,14 @@ struct stats dx_show_entries(struct dx_hash_info *hinfo, struct inode *dir, unsigned count = dx_get_count(entries), names = 0, space = 0, i; unsigned bcount = 0; struct buffer_head *bh; - printk("%i indexed blocks...\n", count); + printk(KERN_WARNING "%i indexed blocks...\n", count); for (i = 0; i < count; i++, entries++) { ext4_lblk_t block = dx_get_block(entries); ext4_lblk_t hash = i ? dx_get_hash(entries): 0; u32 range = i < count - 1? (dx_get_hash(entries + 1) - hash): ~hash; struct stats stats; - printk("%s%3u:%03u hash %8x/%8x ",levels?"":" ", i, block, hash, range); + printk(KERN_WARNING "%s%3u:%03u hash %8x/%8x ",levels?"":" ", i, block, hash, range); bh = ext4_bread(NULL,dir, block, 0); if (!bh || IS_ERR(bh)) continue; @@ -855,7 +855,7 @@ dx_probe(struct ext4_filename *fname, struct inode *dir, goto fail; } - dxtrace(printk("Look up %x", hash)); + dxtrace(printk(KERN_WARNING "Look up %x", hash)); level = 0; blocks[0] = 0; while (1) { diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 450c918d68fc..f1f0427b55a6 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -941,9 +941,9 @@ void __ext4_msg(struct super_block *sb, vaf.fmt = fmt; vaf.va = &args; if (sb) - printk("%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); + printk(KERN_WARNING "%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf); else - printk("%sEXT4-fs: %pV\n", prefix, &vaf); + printk(KERN_WARNING "%sEXT4-fs: %pV\n", prefix, &vaf); va_end(args); }
printk() should include KERN_<LEVEL> facility level, add them. Signed-off-by: Yu Zhe <yuzhe@nfschina.com> --- fs/ext4/inode.c | 2 +- fs/ext4/namei.c | 14 +++++++------- fs/ext4/super.c | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-)