udf : enable error print in udf_read_tagged().
Subsystems (2)
PATCH v3 1/2 udf : enable error print in udf_read_tagged().
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef udf_debug.
If there is a problem with scratched disc or loader, the user needs to know which error happened.
And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
--- fs/udf/directory.c | 4 ++-- fs/udf/inode.c | 14 +++++++------- fs/udf/misc.c | 17 ++++++++++------- fs/udf/super.c | 2 +- fs/udf/truncate.c | 5 ++--- fs/udf/udfdecl.h | 15 +++++++-------- fs/udf/udftime.c | 2 +- fs/udf/unicode.c | 6 +++--- 8 files changed, 33 insertions(+), 32 deletions(-)
fs/udf/directory.c +2 -2
@@ -201,7 +201,7 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offsstruct short_ad *sa;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_fileshortad() invalidparms\n" ); +pr_err ("udf_get_fileshortad() invalidparms\n" );return NULL ; }
@@ -223,7 +223,7 @@ struct long_ad *udf_get_filelongad(uint8_t *ptr, int maxoffset, uint32_t *offsetstruct long_ad *la;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_filelongad() invalidparms\n" ); +pr_err ("udf_get_filelongad() invalidparms\n" );return NULL ; }
fs/udf/inode.c +7 -7
@@ -83,7 +83,7 @@ void udf_evict_inode(struct inode *inode)end_writeback (inode);if (iinfo->i_alloc_type !=ICBTAG_FLAG_AD_IN_ICB && inode->i_size != iinfo->i_lenExtents) { - printk(KERN_WARNING "UDF-fs (%s): Inode %lu (mode %o) has " + pr_warn("(%s): Inode %lu (mode %o) has " "inode size %llu different from extent length %llu. " "Filesystem need not be standards compliant.\n", inode->i_sb->s_id, inode->i_ino, inode->i_mode,
@@ -1169,7 +1169,7 @@ static void __udf_read_inode(struct inode *inode)*/ bh =udf_read_ptagged (inode->i_sb, &iinfo->i_location,0 , &ident);if (!bh) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) failed !bh\n" , +pr_err ("udf_read_inode(ino %ld) failed !bh\n" , inode->i_ino);make_bad_inode (inode);return ;
@@ -1177,7 +1177,7 @@ static void __udf_read_inode(struct inode *inode)if (ident !=TAG_IDENT_FE && ident !=TAG_IDENT_EFE && ident !=TAG_IDENT_USE ) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) " +pr_err ("udf_read_inode(ino %ld) " "failed ident=%d\n" , inode->i_ino, ident);brelse (bh);make_bad_inode (inode);
@@ -1218,7 +1218,7 @@ static void __udf_read_inode(struct inode *inode) }brelse (ibh); }else if (fe->icbTag.strategyType !=cpu_to_le16 (4 )) { -printk (KERN_ERR "udf: unsupported strategy type: %d\n" , +pr_err ("unsupported strategy type: %d\n" ,le16_to_cpu (fe->icbTag.strategyType));brelse (bh);make_bad_inode (inode);
@@ -1413,7 +1413,7 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)udf_debug ("METADATA BITMAP FILE-----\n" );break ;default : -printk (KERN_ERR "udf: udf_fill_inode(ino %ld) failed unknown " +pr_err ("udf_fill_inode(ino %ld) failed unknown " "file type=%d\n" , inode->i_ino, fe->icbTag.fileType);make_bad_inode (inode);
@@ -1438,7 +1438,7 @@ static int udf_alloc_i_data(struct inode *inode, size_t size) iinfo->i_ext.i_data =kmalloc (size,GFP_KERNEL );if (!iinfo->i_ext.i_data) { -printk (KERN_ERR "udf:udf_alloc_i_data (ino %ld) " +pr_err ("udf_alloc_i_data (ino %ld) " "no free memory\n" , inode->i_ino);return -ENOMEM ; }
@@ -1689,7 +1689,7 @@ out:if (do_sync) {sync_dirty_buffer (bh);if (buffer_write_io_error (bh)) { -printk (KERN_WARNING "IO error syncing udf inode " +pr_warn ("IO error syncing udf inode " "[%s:%08lx]\n" , inode->i_sb->s_id, inode->i_ino); err = -EIO ;
fs/udf/misc.c +10 -7
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )
@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , +pr_err ("block=%d, location=%d: read failed\n" , block, location);return NULL ; }
@@ -221,22 +222,24 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_debug ("location mismatch block %u, tag %u != %u\n" , +pr_err ("location mismatch block %u, tag %u != %u\n" , block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +pr_err ("tag checksum failed block %d, checksum 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , -le16_to_cpu (tag_p->descVersion), block); +pr_err ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , +le16_to_cpu (tag_p->descVersion), block); goto error_out; }
@@ -247,7 +250,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_debug ("Crc failure block %d: crc = %d, crclen = %d\n" , block, +pr_err ("Crc failure block %d: crc = %d, crclen = %d\n" , block,le16_to_cpu (tag_p->descCRC),le16_to_cpu (tag_p->descCRCLength)); error_out:
fs/udf/super.c +1 -1
@@ -550,7 +550,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt, uopt->dmode = option &0777 ;break ;default : -printk (KERN_ERR "udf: bad mount option \"%s\" " +pr_err ("bad mount option \"%s\" " "or missing value\n" , p);return 0 ; }
fs/udf/truncate.c +2 -3
@@ -95,8 +95,7 @@ void udf_truncate_tail_extent(struct inode *inode) lbcount += elen;if (lbcount > inode->i_size) {if (lbcount - inode->i_size >= inode->i_sb->s_blocksize) - printk(KERN_WARNING - "udf_truncate_tail_extent(): Too long " + pr_warn("udf_truncate_tail_extent(): Too long " "extent after EOF in inode %u: i_size: " "%Ld lbcount: %Ld extent %u+%u\n", (unsigned)inode->i_ino,
@@ -109,7 +108,7 @@ void udf_truncate_tail_extent(struct inode *inode)extent_trunc (inode, &epos, &eloc, etype, elen, nelen); epos.offset += adsize;if (udf_next_aext (inode, &epos, &eloc, &elen,1 ) !=-1 ) -printk (KERN_ERR "udf_truncate_tail_extent(): " +pr_err ("udf_truncate_tail_extent(): " "Extent after EOF in inode %u.\n" , (unsigned )inode->i_ino);break ;
fs/udf/udfdecl.h +7 -8
@@ -1,6 +1,8 @@#ifndef __UDF_DECL_H#define __UDF_DECL_H +#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include "ecma_167.h" #include "osta_udf.h"
@@ -19,18 +21,15 @@#undef UDFFS_DEBUG#ifdef UDFFS_DEBUG -#define udf_debug(f, a...) \ -do { \ - printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \ - __FILE__, __LINE__, __func__); \ - printk(f, ##a); \ -} while (0) +#define udf_debug (fmt, ...) \ + pr_debug("DEBUG %s:%d:%s: " fmt, \ + __FILE__, __LINE__, __func__, ##__VA_ARGS__);#else -#define udf_debug(f, a...) /**/ +#define udf_debug (fmt, ...)/**/ #endif #define udf_info(f, a...) \ - printk(KERN_INFO "UDF-fs INFO " f, ##a); + pr_info("INFO " f, ##a);#define udf_fixed_to_variable (x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )
fs/udf/udftime.c +1 -1
@@ -34,9 +34,9 @@* http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm */ +#include "udfdecl.h" #include <linux/types.h> #include <linux/kernel.h> -#include "udfdecl.h" #define EPOCH_YEAR 1970
fs/udf/unicode.c +3 -3
@@ -114,7 +114,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i) cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }
@@ -242,7 +242,7 @@ try_again:if (utf_cnt) { error_out: ocu[++u_len] ='?' ; -printk (KERN_DEBUG "udf: bad UTF-8 character\n" ); +pr_debug ("bad UTF-8 character\n" ); } ocu[length -1 ] = (uint8_t )u_len +1 ;
@@ -267,7 +267,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o, cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }
Signature · 1 line
1.7.4.4
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.
Thanks for the patch. I'm willing to take the patch since it's an
improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functions udf_info, udf_warn, udf_err which also print sb->s_id with each error so that user can better identify on which filesystem error happened. So the format would be like:
UDF-fs error/warning/info (device %s): ...
Would you like to improve the patch like this?
Honza
Quoted 284 lines
Signed-off-by Namjae Jeon <[email protected]>--- fs/udf/directory.c | 4 ++-- fs/udf/inode.c | 14 +++++++------- fs/udf/misc.c | 17 ++++++++++------- fs/udf/super.c | 2 +- fs/udf/truncate.c | 5 ++--- fs/udf/udfdecl.h | 15 +++++++-------- fs/udf/udftime.c | 2 +- fs/udf/unicode.c | 6 +++--- 8 files changed, 33 insertions(+), 32 deletions(-)fs/udf/directory.c +2
-2@@ -201,7 +201,7 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offsstruct short_ad *sa;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_fileshortad() invalidparms\n" ); +pr_err ("udf_get_fileshortad() invalidparms\n" );return NULL ; }@@ -223,7 +223,7 @@ struct long_ad *udf_get_filelongad(uint8_t *ptr, int maxoffset, uint32_t *offsetstruct long_ad *la;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_filelongad() invalidparms\n" ); +pr_err ("udf_get_filelongad() invalidparms\n" );return NULL ; }fs/udf/inode.c +7
-7@@ -83,7 +83,7 @@ void udf_evict_inode(struct inode *inode)end_writeback (inode);if (iinfo->i_alloc_type !=ICBTAG_FLAG_AD_IN_ICB && inode->i_size != iinfo->i_lenExtents) { - printk(KERN_WARNING "UDF-fs (%s): Inode %lu (mode %o) has " + pr_warn("(%s): Inode %lu (mode %o) has " "inode size %llu different from extent length %llu. " "Filesystem need not be standards compliant.\n", inode->i_sb->s_id, inode->i_ino, inode->i_mode,@@ -1169,7 +1169,7 @@ static void __udf_read_inode(struct inode *inode)*/ bh =udf_read_ptagged (inode->i_sb, &iinfo->i_location,0 , &ident);if (!bh) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) failed !bh\n" , +pr_err ("udf_read_inode(ino %ld) failed !bh\n" , inode->i_ino);make_bad_inode (inode);return ;@@ -1177,7 +1177,7 @@ static void __udf_read_inode(struct inode *inode)if (ident !=TAG_IDENT_FE && ident !=TAG_IDENT_EFE && ident !=TAG_IDENT_USE ) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) " +pr_err ("udf_read_inode(ino %ld) " "failed ident=%d\n" , inode->i_ino, ident);brelse (bh);make_bad_inode (inode);@@ -1218,7 +1218,7 @@ static void __udf_read_inode(struct inode *inode) }brelse (ibh); }else if (fe->icbTag.strategyType !=cpu_to_le16 (4 )) { -printk (KERN_ERR "udf: unsupported strategy type: %d\n" , +pr_err ("unsupported strategy type: %d\n" ,le16_to_cpu (fe->icbTag.strategyType));brelse (bh);make_bad_inode (inode);@@ -1413,7 +1413,7 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)udf_debug ("METADATA BITMAP FILE-----\n" );break ;default : -printk (KERN_ERR "udf: udf_fill_inode(ino %ld) failed unknown " +pr_err ("udf_fill_inode(ino %ld) failed unknown " "file type=%d\n" , inode->i_ino, fe->icbTag.fileType);make_bad_inode (inode);@@ -1438,7 +1438,7 @@ static int udf_alloc_i_data(struct inode *inode, size_t size) iinfo->i_ext.i_data =kmalloc (size,GFP_KERNEL );if (!iinfo->i_ext.i_data) { -printk (KERN_ERR "udf:udf_alloc_i_data (ino %ld) " +pr_err ("udf_alloc_i_data (ino %ld) " "no free memory\n" , inode->i_ino);return -ENOMEM ; }@@ -1689,7 +1689,7 @@ out:if (do_sync) {sync_dirty_buffer (bh);if (buffer_write_io_error (bh)) { -printk (KERN_WARNING "IO error syncing udf inode " +pr_warn ("IO error syncing udf inode " "[%s:%08lx]\n" , inode->i_sb->s_id, inode->i_ino); err = -EIO ;fs/udf/misc.c +10
-7@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , +pr_err ("block=%d, location=%d: read failed\n" , block, location);return NULL ; }@@ -221,22 +222,24 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_debug ("location mismatch block %u, tag %u != %u\n" , +pr_err ("location mismatch block %u, tag %u != %u\n" , block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +pr_err ("tag checksum failed block %d, checksum 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , -le16_to_cpu (tag_p->descVersion), block); +pr_err ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , +le16_to_cpu (tag_p->descVersion), block); goto error_out; }@@ -247,7 +250,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_debug ("Crc failure block %d: crc = %d, crclen = %d\n" , block, +pr_err ("Crc failure block %d: crc = %d, crclen = %d\n" , block,le16_to_cpu (tag_p->descCRC),le16_to_cpu (tag_p->descCRCLength)); error_out:fs/udf/super.c +1
-1@@ -550,7 +550,7 @@ static int udf_parse_options(char *options, struct udf_options *uopt, uopt->dmode = option &0777 ;break ;default : -printk (KERN_ERR "udf: bad mount option \"%s\" " +pr_err ("bad mount option \"%s\" " "or missing value\n" , p);return 0 ; }fs/udf/truncate.c +2
-3@@ -95,8 +95,7 @@ void udf_truncate_tail_extent(struct inode *inode) lbcount += elen;if (lbcount > inode->i_size) {if (lbcount - inode->i_size >= inode->i_sb->s_blocksize) - printk(KERN_WARNING - "udf_truncate_tail_extent(): Too long " + pr_warn("udf_truncate_tail_extent(): Too long " "extent after EOF in inode %u: i_size: " "%Ld lbcount: %Ld extent %u+%u\n", (unsigned)inode->i_ino,@@ -109,7 +108,7 @@ void udf_truncate_tail_extent(struct inode *inode)extent_trunc (inode, &epos, &eloc, etype, elen, nelen); epos.offset += adsize;if (udf_next_aext (inode, &epos, &eloc, &elen,1 ) !=-1 ) -printk (KERN_ERR "udf_truncate_tail_extent(): " +pr_err ("udf_truncate_tail_extent(): " "Extent after EOF in inode %u.\n" , (unsigned )inode->i_ino);break ;fs/udf/udfdecl.h +7
-8@@ -1,6 +1,8 @@#ifndef __UDF_DECL_H#define __UDF_DECL_H +#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include "ecma_167.h" #include "osta_udf.h" @@ -19,18 +21,15 @@#undef UDFFS_DEBUG#ifdef UDFFS_DEBUG -#define udf_debug(f, a...) \ -do { \ - printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \ - __FILE__, __LINE__, __func__); \ - printk(f, ##a); \ -} while (0) +#define udf_debug (fmt, ...) \ + pr_debug("DEBUG %s:%d:%s: " fmt, \ + __FILE__, __LINE__, __func__, ##__VA_ARGS__);#else -#define udf_debug(f, a...) /**/ +#define udf_debug (fmt, ...)/**/ #endif #define udf_info(f, a...) \ - printk(KERN_INFO "UDF-fs INFO " f, ##a); + pr_info("INFO " f, ##a);#define udf_fixed_to_variable (x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )fs/udf/udftime.c +1
-1@@ -34,9 +34,9 @@* http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm */ +#include "udfdecl.h" #include <linux/types.h> #include <linux/kernel.h> -#include "udfdecl.h" #define EPOCH_YEAR 1970fs/udf/unicode.c +3
-3@@ -114,7 +114,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i) cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }@@ -242,7 +242,7 @@ try_again:if (utf_cnt) { error_out: ocu[++u_len] ='?' ; -printk (KERN_DEBUG "udf: bad UTF-8 character\n" ); +pr_debug ("bad UTF-8 character\n" ); } ocu[length -1 ] = (uint8_t )u_len +1 ;@@ -267,7 +267,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o, cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }Signature · 1 line
1.7.4.4
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 10 lines · Jan Kara
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.
Quoted 5 lines
Thanks for the patch. I'm willing to take the patch since it's an
improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functionsudf_info,udf_warn,udf_errwhich also printsb->s_idwith each error so that user can better identify on which filesystem error happened.
Modifying the follow-on patch I sent would be pretty trivial.
Quoted 11 lines · Joe Perches
Quoted 9 lines · Jan Kara
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.
Quoted 7 lines
Quoted 5 lines
Thanks for the patch. I'm willing to take the patch since it's an
improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functionsudf_info,udf_warn,udf_errwhich also printsb->s_idwith each error so that user can better identify on which filesystem error happened.Modifying the follow-on patch I sent would be pretty trivial.
Sure, I can do that but if Namjae is willing to do it, I'd have less work
:).
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
2011/10/7 Jan Kara <[email protected]>:
Quoted 18 lines · Joe Perches
Quoted 17 lines · Jan Kara
Quoted 15 lines
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.Thanks for the patch. I'm willing to take the patch since it's an improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functions
udf_info,udf_warn,udf_errwhich also printsb->s_idwith each error so that user can better identify on which filesystem error happened.
Quoted 3 lines
Quoted 1 line
Modifying the follow-on patch I sent would be pretty trivial.
Sure, I can do that but if Namjae is willing to do it, I'd have less work :).
Hi. Jan.
Currently I can not send a patch using git send-email. maybe it is the problem related with git or patch size... So I attach a patch. Is it okay? or I resend a patch again.. plz review attach patch.
Thanks.
Quoted 6 lines
Honza --
Jan Kara <[email protected]>
SUSE Labs, CR
From 1f7d2991e18050d8230aa7b563d61216918bd865 Mon Sep 17 00:00:00 2001
From: Namjae Jeon <[email protected]>
Date: Sat, 8 Oct 2011 23:32:03 +0900
Subject: [PATCH v4] udf : enable error print in udf_read_tagged().
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef udf_debug.
If there is a problem with scratched disc or loader, the user needs to know which error happened.
And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
--- fs/udf/balloc.c | 15 ++-- fs/udf/directory.c | 24 +++--- fs/udf/inode.c | 44 +++++----- fs/udf/lowlevel.c | 2 +- fs/udf/misc.c | 27 ++++-- fs/udf/namei.c | 8 +- fs/udf/partition.c | 8 +- fs/udf/super.c | 237 ++++++++++++++++++++++++++++------------------------ fs/udf/truncate.c | 25 +++--- fs/udf/udfdecl.h | 40 +++++---- fs/udf/udftime.c | 2 +- fs/udf/unicode.c | 36 ++++---- 12 files changed, 255 insertions(+), 213 deletions(-)
fs/udf/balloc.c +8 -7
@@ -59,8 +59,8 @@ static int __load_block_bitmap(struct super_block *sb,int nr_groups = bitmap->s_nr_groups;if (block_group >= nr_groups) { -udf_debug ("block_group (%d) > nr_groups (%d)\n" , block_group, - nr_groups); +udf_debug ("block_group (%d) > nr_groups (%d)\n" , + block_group, nr_groups); }if (bitmap->s_block_bitmap[block_group]) {
@@ -126,8 +126,9 @@ static void udf_bitmap_free_blocks(struct super_block *sb,if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) {udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, - count, partmap->s_partition_len); + bloc->logicalBlockNum,0 , + bloc->logicalBlockNum, count, + partmap->s_partition_len); goto error_return; }
@@ -369,8 +370,8 @@ static void udf_table_free_blocks(struct super_block *sb,if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) {udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, count, - partmap->s_partition_len); + bloc->logicalBlockNum,0 , bloc->logicalBlockNum, count, + partmap->s_partition_len); goto error_return; }
@@ -611,7 +612,7 @@ static int udf_table_prealloc_blocks(struct super_block *sb,while (first_block != eloc.logicalBlockNum && (etype =udf_next_aext (table, &epos, &eloc, &elen,1 )) !=-1 ) {udf_debug ("eloc=%d, elen=%d, first_block=%d\n" , - eloc.logicalBlockNum, elen, first_block); + eloc.logicalBlockNum, elen, first_block); ;/* empty loop body */ }
fs/udf/directory.c +13 -11
@@ -162,8 +162,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)int padlen;if ((!buffer) || (!offset)) { -udf_debug ("invalidparms\n, buffer=%p, offset=%p\n" , buffer, - offset); +udf_debug ("invalidparms, buffer=%p, offset=%p\n" , + buffer, offset);return NULL ; }
@@ -174,10 +174,9 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset) fi = (struct fileIdentDesc *)ptr;if (fi->descTag.tagIdent !=cpu_to_le16 (TAG_IDENT_FID )) {udf_debug ("0x%x != TAG_IDENT_FID\n" , -le16_to_cpu (fi->descTag.tagIdent)); -udf_debug ("offset: %u sizeof: %lu bufsize: %u\n" , - *offset, (unsigned long )sizeof (struct fileIdentDesc ), - bufsize); +le16_to_cpu (fi->descTag.tagIdent)); +udf_debug ("offset: %u sizeof: %zu bufsize: %u\n" , + *offset,sizeof (struct fileIdentDesc ), bufsize);return NULL ; }if ((*offset +sizeof (struct fileIdentDesc )) > bufsize)
@@ -195,13 +194,14 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)return fi; } -struct short_ad *udf_get_fileshortad (uint8_t *ptr,int maxoffset,uint32_t *offset, -int inc) +struct short_ad *udf_get_fileshortad (struct inode *inode,uint8_t *ptr, +int maxoffset,uint32_t *offset,int inc) {struct short_ad *sa;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_fileshortad() invalidparms\n" ); +udf_err (inode->i_sb, __func__,"udf_get_fileshortad() " +"invalidparms\n" );return NULL ; }
@@ -218,12 +218,14 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offsreturn sa; } -struct long_ad *udf_get_filelongad (uint8_t *ptr,int maxoffset,uint32_t *offset,int inc) +struct long_ad *udf_get_filelongad (struct inode *inode,uint8_t *ptr, +int maxoffset,uint32_t *offset,int inc) {struct long_ad *la;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_filelongad() invalidparms\n" ); +udf_err (inode->i_sb, __func__,"udf_get_filelongad() " +"invalidparms\n" );return NULL ; }
fs/udf/inode.c +24 -20
112 lines not on this page · raw message
fs/udf/lowlevel.c +1 -1
@@ -38,7 +38,7 @@ unsigned int udf_get_last_session(struct super_block *sb)if (i ==0 ) {udf_debug ("XA disk: %s, vol_desc_start=%d\n" , - (ms_info.xa_flag ?"yes" :"no" ), ms_info.addr.lba); + (ms_info.xa_flag ?"yes" :"no" ), ms_info.addr.lba);if (ms_info.xa_flag)/* necessary for a valid ms_info.addr */ vol_desc_start = ms_info.addr.lba; }else {
fs/udf/misc.c +17 -10
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )
@@ -211,8 +212,8 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , - block, location); +udf_err (sb, __func__,"block=%d, location=%d: read failed\n" , + block, location);return NULL ; }
@@ -221,22 +222,27 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_debug ("location mismatch block %u, tag %u != %u\n" , - block,le32_to_cpu (tag_p->tagLocation), location); +udf_err (sb, __func__,"location mismatch block %u, " +"tag %u != %u\n" , + block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +udf_err (sb, __func__,"tag checksum failed block %d, " +"checksum 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , -le16_to_cpu (tag_p->descVersion), block); +udf_err (sb, __func__,"tag version 0x%04x != 0x0002 || " +"0x0003 block %d\n" , +le16_to_cpu (tag_p->descVersion), block); goto error_out; }
@@ -247,8 +253,9 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_debug ("Crc failure block %d: crc = %d, crclen = %d\n" , block, -le16_to_cpu (tag_p->descCRC),le16_to_cpu (tag_p->descCRCLength)); +udf_err (sb, __func__,"Crc failure block %d: crc = %d, crclen = %d\n" , + block,le16_to_cpu (tag_p->descCRC), +le16_to_cpu (tag_p->descCRCLength)); error_out:brelse (bh);
fs/udf/namei.c +4 -4
@@ -799,9 +799,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)if (retval) goto end_rmdir;if (inode->i_nlink !=2 ) -udf_warning (inode->i_sb,"udf_rmdir" , -"empty directory has nlink != 2 (%d)" , - inode->i_nlink); +udf_warn (inode->i_sb, __func__,"udf_rmdir" +"empty directory has nlink != 2 (%d)\n" , + inode->i_nlink);clear_nlink (inode); inode->i_size =0 ;inode_dec_link_count (dir);
@@ -839,7 +839,7 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry)if (!inode->i_nlink) {udf_debug ("Deleting nonexistent file (%lu), %d\n" , - inode->i_ino, inode->i_nlink); + inode->i_ino, inode->i_nlink); inode->i_nlink =1 ; } retval =udf_delete_entry (dir, fi, &fibh, &cfi);
fs/udf/partition.c +5 -3
@@ -34,7 +34,8 @@ uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,struct udf_part_map *map;if (partition >= sbi->s_partitions) {udf_debug ("block=%d, partition=%d, offset=%d: " -"invalid partition\n" , block, partition, offset); +"invalid partition\n" , + block, partition, offset);return 0xFFFFFFFF ; } map = &sbi->s_partmaps[partition];
@@ -61,7 +62,8 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block,if (block > vdata->s_num_entries) {udf_debug ("Trying to access block beyond end of VAT " -"(%d max %d)\n" , block, vdata->s_num_entries); +"(%d max %d)\n" , + block, vdata->s_num_entries);return 0xFFFFFFFF ; }
@@ -322,7 +324,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,BUG_ON (!inode); retblk =udf_try_read_meta (inode, block, partition, offset);if (retblk ==0xFFFFFFFF ) { -udf_warning (sb, __func__,"error reading from METADATA, " +udf_warn (sb, __func__,"error reading from METADATA, " "trying to read from MIRROR" ); inode = mdata->s_mirror_fe;if (!inode)
fs/udf/super.c +126 -111
570 lines not on this page · raw message
fs/udf/truncate.c +13 -12
@@ -95,23 +95,24 @@ void udf_truncate_tail_extent(struct inode *inode) lbcount += elen;if (lbcount > inode->i_size) {if (lbcount - inode->i_size >= inode->i_sb->s_blocksize) -printk (KERN_WARNING -"udf_truncate_tail_extent(): Too long " -"extent after EOF in inode %u: i_size: " -"%Ld lbcount: %Ld extent %u+%u\n" , - (unsigned )inode->i_ino, - (long long )inode->i_size, - (long long )lbcount, - (unsigned )eloc.logicalBlockNum, - (unsigned )elen); +udf_warn (inode->i_sb, __func__, +"udf_truncate_tail_extent(): Too long " +"extent after EOF in inode %u: i_size: " +"%Ld lbcount: %Ld extent %u+%u\n" , + (unsigned )inode->i_ino, + (long long )inode->i_size, + (long long )lbcount, + (unsigned )eloc.logicalBlockNum, + (unsigned )elen); nelen = elen - (lbcount - inode->i_size); epos.offset -= adsize;extent_trunc (inode, &epos, &eloc, etype, elen, nelen); epos.offset += adsize;if (udf_next_aext (inode, &epos, &eloc, &elen,1 ) !=-1 ) -printk (KERN_ERR "udf_truncate_tail_extent(): " -"Extent after EOF in inode %u.\n" , - (unsigned )inode->i_ino); +udf_warn (inode->i_sb, __func__, +"udf_truncate_tail_extent(): " +"Extent after EOF in inode %u.\n" , + (unsigned )inode->i_ino);break ; } }
fs/udf/udfdecl.h +24 -16
@@ -1,6 +1,8 @@#ifndef __UDF_DECL_H#define __UDF_DECL_H +#define pr_fmt (fmt) "UDF-fs " fmt +#include "ecma_167.h" #include "osta_udf.h"
@@ -16,23 +18,28 @@#define UDF_PREALLOCATE #define UDF_DEFAULT_PREALLOC_BLOCKS 8 +__attribute__((format (printf,3 ,4 ))) +void udf_err (struct super_block *sb,const char *function, +const char *fmt, ...); + +__attribute__((format (printf,3 ,4 ))) +void udf_warn (struct super_block *sb,const char *function, +const char *fmt, ...); + +__attribute__((format (printf,2 ,3 ))) +void udf_info (struct super_block *sb,const char *fmt, ...); +#undef UDFFS_DEBUG#ifdef UDFFS_DEBUG -#define udf_debug(f, a...) \ -do { \ - printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \ - __FILE__, __LINE__, __func__); \ - printk(f, ##a); \ -} while (0) +#define udf_debug (fmt, ...) \ + pr_debug("debug %s:%d:%s: " fmt, \ + __FILE__, __LINE__, __func__, ##__VA_ARGS__)#else -#define udf_debug(f, a...) /**/ +#define udf_debug (fmt, ...) \ + no_printk(KERN_DEBUG fmt, ##__VA_ARGS__)#endif -#define udf_info(f, a...) \ - printk(KERN_INFO "UDF-fs INFO " f, ##a); - -#define udf_fixed_to_variable (x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )#define udf_variable_to_fixed (x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
@@ -112,8 +119,6 @@ struct extent_position {/* super.c */ -__attribute__((format (printf,3 ,4 ))) -extern void udf_warning (struct super_block *,const char *,const char *, ...);static inline void udf_updated_lvid (struct super_block *sb) {struct buffer_head *bh =UDF_SB (sb)->s_lvid_bh;
@@ -202,7 +207,8 @@ extern int udf_get_filename(struct super_block *, uint8_t *, uint8_t *, int);extern int udf_put_filename (struct super_block *,const uint8_t *,uint8_t *,int );extern int udf_build_ustr (struct ustr *,dstring *,int ); -extern int udf_CS0toUTF8 (struct ustr *,const struct ustr *); +extern int udf_CS0toUTF8 (struct super_block *,struct ustr *, +const struct ustr *);/* ialloc.c */ extern void udf_free_inode (struct inode *);
@@ -230,8 +236,10 @@ extern struct fileIdentDesc *udf_fileident_read(struct inode *, loff_t *, sector_t *);extern struct fileIdentDesc *udf_get_fileident (void *buffer,int bufsize,int *offset); -extern struct long_ad *udf_get_filelongad (uint8_t *,int ,uint32_t *,int ); -extern struct short_ad *udf_get_fileshortad (uint8_t *,int ,uint32_t *,int ); +extern struct long_ad *udf_get_filelongad (struct inode *,uint8_t *,int , +uint32_t *,int ); +extern struct short_ad *udf_get_fileshortad (struct inode *,uint8_t *,int , +uint32_t *,int );/* udftime.c */ extern struct timespec *udf_disk_stamp_to_time(struct timespec *dest,
fs/udf/udftime.c +1 -1
@@ -34,9 +34,9 @@* http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm */ +#include "udfdecl.h" #include <linux/types.h> #include <linux/kernel.h> -#include "udfdecl.h" #define EPOCH_YEAR 1970
fs/udf/unicode.c +19 -17
@@ -99,7 +99,8 @@ static int udf_build_ustr_exact(struct ustr *dest, dstring *ptr, int exactsize)* November 12, 1997 - Andrew E. Mileski * Written, tested, and released. */ -int udf_CS0toUTF8 (struct ustr *utf_o,const struct ustr *ocu_i) +int udf_CS0toUTF8 (struct super_block *sb,struct ustr *utf_o, +const struct ustr *ocu_i) {const uint8_t *ocu;uint8_t cmp_id, ocu_len;
@@ -114,8 +115,8 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i) cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , - cmp_id, ocu_i->u_name); +udf_err (sb, __func__,"unknown compression code (%d) stri=%s\n" , + cmp_id, ocu_i->u_name);return 0 ; }
@@ -174,7 +175,8 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i)* November 12, 1997 - Andrew E. Mileski * Written, tested, and released. */ -static int udf_UTF8toCS0 (dstring *ocu,struct ustr *utf,int length) +static int udf_UTF8toCS0 (struct super_block *sb,dstring *ocu,struct ustr *utf, +int length) {unsigned c, i, max_val, utf_char;int utf_cnt, u_len;
@@ -242,7 +244,7 @@ try_again:if (utf_cnt) { error_out: ocu[++u_len] ='?' ; -printk (KERN_DEBUG "udf: bad UTF-8 character\n" ); +udf_debug ("bad UTF-8 character\n" ); } ocu[length -1 ] = (uint8_t )u_len +1 ;
@@ -250,8 +252,8 @@ error_out:return u_len +1 ; } -static int udf_CS0toNLS (struct nls_table *nls,struct ustr *utf_o, -const struct ustr *ocu_i) +static int udf_CS0toNLS (struct super_block *sb,struct nls_table *nls, +struct ustr *utf_o,const struct ustr *ocu_i) {const uint8_t *ocu;uint8_t cmp_id, ocu_len;
@@ -267,8 +269,8 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o, cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , - cmp_id, ocu_i->u_name); +udf_err (sb, __func__,"unknown compression code (%d) stri=%s\n" , + cmp_id, ocu_i->u_name);return 0 ; }
@@ -293,8 +295,8 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o,return utf_o->u_len; } -static int udf_NLStoCS0 (struct nls_table *nls,dstring *ocu,struct ustr *uni, -int length) +static int udf_NLStoCS0 (struct super_block *sb,struct nls_table *nls, +dstring *ocu,struct ustr *uni,int length) {int len;unsigned i, max_val;
@@ -351,16 +353,16 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, uint8_t *dname, goto out2;if (UDF_QUERY_FLAG (sb,UDF_FLAG_UTF8 )) { -if (!udf_CS0toUTF8 (filename, unifilename)) { +if (!udf_CS0toUTF8 (sb, filename, unifilename)) {udf_debug ("Failed in udf_get_filename: sname = %s\n" , - sname); + sname); goto out2; } }else if (UDF_QUERY_FLAG (sb,UDF_FLAG_NLS_MAP )) { -if (!udf_CS0toNLS (UDF_SB (sb)->s_nls_map, filename, +if (!udf_CS0toNLS (sb,UDF_SB (sb)->s_nls_map, filename, unifilename)) {udf_debug ("Failed in udf_get_filename: sname = %s\n" , - sname); + sname); goto out2; } }else
@@ -385,11 +387,11 @@ int udf_put_filename(struct super_block *sb, const uint8_t *sname,return 0 ;if (UDF_QUERY_FLAG (sb,UDF_FLAG_UTF8 )) { - namelen =udf_UTF8toCS0 (dname, &unifilename,UDF_NAME_LEN ); + namelen =udf_UTF8toCS0 (sb, dname, &unifilename,UDF_NAME_LEN );if (!namelen)return 0 ; }else if (UDF_QUERY_FLAG (sb,UDF_FLAG_NLS_MAP )) { - namelen =udf_NLStoCS0 (UDF_SB (sb)->s_nls_map, dname, + namelen =udf_NLStoCS0 (sb,UDF_SB (sb)->s_nls_map, dname, &unifilename,UDF_NAME_LEN );if (!namelen)return 0 ;
Signature · 1 line
1.7.4.4
Quoted 22 lines · NamJae Jeon
2011/10/7 Jan Kara <[email protected]>:
Quoted 18 lines · Joe Perches
Quoted 17 lines · Jan Kara
Quoted 15 lines
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.Thanks for the patch. I'm willing to take the patch since it's an
improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functionsudf_info,udf_warn,udf_errwhich also printsb->s_idwith each error so that user can better identify on which filesystem error happened.Quoted 3 lines
Quoted 1 line
Modifying the follow-on patch I sent would be pretty trivial.
Sure, I can do that but if Namjae is willing to do it, I'd have less work
:).
Quoted 7 lines
Hi. Jan.
Currently I can not send a patch using git send-email. maybe it is the problem related with git or patch size... So I attach a patch. Is it okay? or I resend a patch again.. plz review attach patch.
Thanks.
Not a good patch I think.
You've broken format strings when they should be coalesced and the alignment looks odd.
udf_debug should also add
struct super_block *sb, const char *function,
as the leading arguments.
2011/10/9 Joe Perches <[email protected]>:
Quoted 23 lines · NamJae Jeon
Quoted 22 lines
2011/10/7 Jan Kara <[email protected]>:
Quoted 21 lines · Joe Perches
Quoted 11 lines · Jan Kara
Quoted 9 lines
Hello,
Quoted 6 lines · Namjae Jeon
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef
udf_debug. If there is a problem with scratched disc or loader, the user needs to know which error happened. And I usepr_fmtinstead of printk by joe's suggestion. I try to modify totally it topr_fmtalso.Quoted 7 lines
Quoted 5 lines
Thanks for the patch. I'm willing to take the patch since it's an improvement but what would be even nicer is to have error reporting like in ext3 / ext4. We would have functions
udf_info,udf_warn,udf_errwhich also printsb->s_idwith each error so that user can better identify on which filesystem error happened.Modifying the follow-on patch I sent would be pretty trivial.
Sure, I can do that but if Namjae is willing to do it, I'd have less work :).
Quoted 8 lines
Quoted 7 lines
Hi. Jan.
Currently I can not send a patch using git send-email. maybe it is the problem related with git or patch size... So I attach a patch. Is it okay? or I resend a patch again.. plz review attach patch.
Thanks.
Hi. Joe.
Quoted 4 lines
Not a good patch I think.
You've broken format strings when they should be coalesced and the alignment looks odd.
Would you explain more ? I didn't understand your meaning yet.
Quoted 4 lines
udf_debugshould also add
structsuper_block*sb, const char *function, as the leading arguments.
okay. I will~
Thanks for your review.
Quoted 2 lines
Quoted 5 lines · NamJae Jeon
2011/10/9 Joe Perches <[email protected]>:
Quoted 3 lines
Not a good patch I think.
You've broken format strings when they should be coalesced and the alignment looks odd.Would you explain more ? I didn't understand your meaning yet.
Format strings are allowed to exceed 80 columns. Arguments should be aligned to the open parenthesis
For example:
pr_err("some really long format string longer than "
"80 columns and not broken into pieces, Count: %d\n", count);
is better as:
pr_err("some really long format string longer than 80 columns and not broken into pieces, Count: %d\n",
count);Coalesced format strings makes dmesg output easier to grep. count is not indented by 2 tabs, but 1 tab and 7 spaces.
There's no hard and fast rule on the indentation.
The patch I sent coalesced all format strings and used that alignment style.
2011/10/9 Joe Perches <[email protected]>:
Quoted 18 lines · NamJae Jeon
Quoted 5 lines
2011/10/9 Joe Perches <[email protected]>:
Quoted 3 lines
Not a good patch I think.
You've broken format strings when they should be coalesced and the alignment looks odd.Would you explain more ? I didn't understand your meaning yet.
Format strings are allowed to exceed 80 columns. Arguments should be aligned to the open parenthesis
For example:
pr_err("some really long format string longer than "
"80 columns and not broken into pieces, Count: %d\n",
count);
is better as:
pr_err("some really long format string longer than 80 columns and not broken into pieces, Count: %d\n", count);
Quoted 7 lines
Coalesced format strings makes dmesg output easier to grep. count is not indented by 2 tabs, but 1 tab and 7 spaces.
There's no hard and fast rule on the indentation.
The patch I sent coalesced all format strings and used that alignment style.
Clear~ I will do it as your guide.
Thanks very much. joe~
Quoted 2 lines
2011/10/9 NamJae Jeon <[email protected]>:
Quoted 22 lines
2011/10/9 Joe Perches <[email protected]>:
Quoted 21 lines · NamJae Jeon
Quoted 5 lines
2011/10/9 Joe Perches <[email protected]>:
Quoted 3 lines
Not a good patch I think.
You've broken format strings when they should be coalesced and the alignment looks odd.Would you explain more ? I didn't understand your meaning yet.
Format strings are allowed to exceed 80 columns. Arguments should be aligned to the open parenthesis
For example:
pr_err("some really long format string longer than "
"80 columns and not broken into pieces, Count: %d\n",
count);
is better as:
pr_err("some really long format string longer than 80 columns and not broken into pieces, Count: %d\n", count);Coalesced format strings makes dmesg output easier to grep. count is not indented by 2 tabs, but 1 tab and 7 spaces.
Quoted 6 lines
Quoted 4 lines
There's no hard and fast rule on the indentation.
The patch I sent coalesced all format strings and used that alignment style.
Clear~ I will do it as your guide.
Thanks very much. joe~
Hi. joe.
I attach v5 patch.
plz review it.
Thanks.
Quoted 3 lines
Quoted 2 lines
From 0fb1239f662c26b80b62241f7a137485ce523f40 Mon Sep 17 00:00:00 2001
From: Namjae Jeon <[email protected]>
Date: Sun, 9 Oct 2011 22:54:51 +0900
Subject: [PATCH v5] udf : enable error print in udf_read_tagged().
While reading metadata, if a problem occurs, Print out only one of the five case.(It also does not show a checksum properly.) Because currently it have been disable by undef udf_debug.
If there is a problem with scratched disc or loader, the user needs to know which error happened.
And I use pr_fmt instead of printk by joe's suggestion. I try to modify totally it to pr_fmt also.
--- fs/udf/balloc.c | 30 +++-- fs/udf/directory.c | 35 +++-- fs/udf/file.c | 11 +- fs/udf/inode.c | 61 +++++---- fs/udf/lowlevel.c | 8 +- fs/udf/misc.c | 28 +++-- fs/udf/namei.c | 15 +- fs/udf/partition.c | 23 ++-- fs/udf/super.c | 389 ++++++++++++++++++++++++++++++---------------------- fs/udf/truncate.c | 25 ++-- fs/udf/udfdecl.h | 43 +++--- fs/udf/udftime.c | 2 +- fs/udf/unicode.c | 42 +++--- 13 files changed, 406 insertions(+), 306 deletions(-)
fs/udf/balloc.c +17 -13
@@ -59,8 +59,8 @@ static int __load_block_bitmap(struct super_block *sb,int nr_groups = bitmap->s_nr_groups;if (block_group >= nr_groups) { -udf_debug ("block_group (%d) > nr_groups (%d)\n" , block_group, - nr_groups); +udf_debug (sb, __func__,"block_group (%d) > nr_groups (%d)\n" , + block_group, nr_groups); }if (bitmap->s_block_bitmap[block_group]) {
@@ -125,9 +125,10 @@ static void udf_bitmap_free_blocks(struct super_block *sb, partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) { -udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, - count, partmap->s_partition_len); +udf_debug (sb, __func__,"%d < %d || %d + %d > %d\n" , + bloc->logicalBlockNum,0 , + bloc->logicalBlockNum, count, + partmap->s_partition_len); goto error_return; }
@@ -153,8 +154,10 @@ static void udf_bitmap_free_blocks(struct super_block *sb, bh = bitmap->s_block_bitmap[bitmap_nr];for (i =0 ; i < count; i++) {if (udf_set_bit (bit + i, bh->b_data)) { -udf_debug ("bit %ld already set\n" , bit + i); -udf_debug ("byte=%2x\n" , +udf_debug (sb, __func__, +"bit %ld already set\n" , + bit + i); +udf_debug (sb, __func__,"byte=%2x\n" , ((char *)bh->b_data)[(bit + i) >>3 ]); } }
@@ -330,7 +333,8 @@ got_block: (sizeof (struct spaceBitmapDesc ) <<3 );if (!udf_clear_bit (bit, bh->b_data)) { -udf_debug ("bit already cleared for block %d\n" , bit); +udf_debug (sb, __func__,"bit already cleared for block %d\n" , + bit); goto repeat; }
@@ -368,9 +372,9 @@ static void udf_table_free_blocks(struct super_block *sb, partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) { -udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, count, - partmap->s_partition_len); +udf_debug (sb, __func__,"%d < %d || %d + %d > %d\n" , + bloc->logicalBlockNum,0 , bloc->logicalBlockNum, count, + partmap->s_partition_len); goto error_return; }
@@ -610,8 +614,8 @@ static int udf_table_prealloc_blocks(struct super_block *sb,while (first_block != eloc.logicalBlockNum && (etype =udf_next_aext (table, &epos, &eloc, &elen,1 )) !=-1 ) { -udf_debug ("eloc=%d, elen=%d, first_block=%d\n" , - eloc.logicalBlockNum, elen, first_block); +udf_debug (sb, __func__,"eloc=%d, elen=%d, first_block=%d\n" , + eloc.logicalBlockNum, elen, first_block); ;/* empty loop body */ }
fs/udf/directory.c +20 -15
@@ -33,7 +33,7 @@ struct fileIdentDesc *udf_fileident_read(struct inode *dir, loff_t *nf_pos, fibh->soffset = fibh->eoffset;if (iinfo->i_alloc_type ==ICBTAG_FLAG_AD_IN_ICB ) { - fi = udf_get_fileident(iinfo->i_ext.i_data - + fi = udf_get_fileident(dir, iinfo->i_ext.i_data - (iinfo->i_efe ? sizeof(struct extendedFileEntry) : sizeof(struct fileEntry)),
@@ -98,7 +98,7 @@ struct fileIdentDesc *udf_fileident_read(struct inode *dir, loff_t *nf_pos, fibh->sbh = fibh->ebh; } - fi =udf_get_fileident (fibh->sbh->b_data, dir->i_sb->s_blocksize, + fi =udf_get_fileident (dir, fibh->sbh->b_data, dir->i_sb->s_blocksize, &(fibh->eoffset));if (!fi)
@@ -154,7 +154,8 @@ struct fileIdentDesc *udf_fileident_read(struct inode *dir, loff_t *nf_pos,return fi; } -struct fileIdentDesc *udf_get_fileident (void *buffer,int bufsize,int *offset) +struct fileIdentDesc *udf_get_fileident (struct inode *inode,void *buffer, +int bufsize,int *offset) {struct fileIdentDesc *fi;int lengthThisIdent;
@@ -162,8 +163,9 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)int padlen;if ((!buffer) || (!offset)) { -udf_debug ("invalidparms\n, buffer=%p, offset=%p\n" , buffer, - offset); +udf_debug (inode->i_sb, __func__, +"invalidparms, buffer=%p, offset=%p\n" , + buffer, offset);return NULL ; }
@@ -173,11 +175,11 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset) ptr += *offset; fi = (struct fileIdentDesc *)ptr;if (fi->descTag.tagIdent !=cpu_to_le16 (TAG_IDENT_FID )) { -udf_debug ("0x%x != TAG_IDENT_FID\n" , -le16_to_cpu (fi->descTag.tagIdent)); -udf_debug ("offset: %u sizeof: %lu bufsize: %u\n" , - *offset, (unsigned long )sizeof (struct fileIdentDesc ), - bufsize); +udf_debug (inode->i_sb, __func__,"0x%x != TAG_IDENT_FID\n" , +le16_to_cpu (fi->descTag.tagIdent)); +udf_debug (inode->i_sb, __func__, +"offset: %u sizeof: %zu bufsize: %u\n" , + *offset,sizeof (struct fileIdentDesc ), bufsize);return NULL ; }if ((*offset +sizeof (struct fileIdentDesc )) > bufsize)
@@ -195,13 +197,14 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)return fi; } -struct short_ad *udf_get_fileshortad (uint8_t *ptr,int maxoffset,uint32_t *offset, -int inc) +struct short_ad *udf_get_fileshortad (struct inode *inode,uint8_t *ptr, +int maxoffset,uint32_t *offset,int inc) {struct short_ad *sa;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_fileshortad() invalidparms\n" ); +udf_err (inode->i_sb, __func__, +"udf_get_fileshortad() invalidparms\n" );return NULL ; }
@@ -218,12 +221,14 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offsreturn sa; } -struct long_ad *udf_get_filelongad (uint8_t *ptr,int maxoffset,uint32_t *offset,int inc) +struct long_ad *udf_get_filelongad (struct inode *inode,uint8_t *ptr, +int maxoffset,uint32_t *offset,int inc) {struct long_ad *la;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_filelongad() invalidparms\n" ); +udf_err (inode->i_sb, __func__, +"udf_get_filelongad() invalidparms\n" );return NULL ; }
fs/udf/file.c +8 -3
@@ -124,7 +124,9 @@ static ssize_t udf_file_aio_write(struct kiocb *iocb, const struct iovec *iov, pos + count)) { err =udf_expand_file_adinicb (inode);if (err) { -udf_debug ("udf_expand_adinicb: err=%d\n" , err); +udf_debug (inode->i_sb, __func__, +"udf_expand_adinicb: err=%d\n" , + err);up_write (&iinfo->i_data_sem);return err; }
@@ -151,13 +153,16 @@ long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)int result = -EINVAL ;if (inode_permission (inode,MAY_READ ) !=0 ) { -udf_debug ("no permission to access inode %lu\n" , inode->i_ino); +udf_debug (inode->i_sb, __func__, +"no permission to access inode %lu\n" , + inode->i_ino); result = -EPERM ; goto out; }if (!arg) { -udf_debug ("invalid argument to udf_ioctl\n" ); +udf_debug (inode->i_sb, __func__, +"invalid argument to udf_ioctl\n" ); result = -EINVAL ; goto out; }
fs/udf/inode.c +34 -27
147 lines not on this page · raw message
fs/udf/lowlevel.c +5 -3
@@ -37,12 +37,14 @@ unsigned int udf_get_last_session(struct super_block *sb) i =ioctl_by_bdev (bdev,CDROMMULTISESSION , (unsigned long )&ms_info);if (i ==0 ) { -udf_debug ("XA disk: %s, vol_desc_start=%d\n" , - (ms_info.xa_flag ?"yes" :"no" ), ms_info.addr.lba); +udf_debug (sb, __func__,"XA disk: %s, vol_desc_start=%d\n" , + (ms_info.xa_flag ?"yes" :"no" ), ms_info.addr.lba);if (ms_info.xa_flag)/* necessary for a valid ms_info.addr */ vol_desc_start = ms_info.addr.lba; }else { -udf_debug ("CDROMMULTISESSION not supported: rc=%d\n" , i); +udf_debug (sb, __func__, +"CDROMMULTISESSION not supported: rc=%d\n" , + i); }return vol_desc_start; }
fs/udf/misc.c +18 -10
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )
@@ -211,8 +212,8 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , - block, location); +udf_err (sb, __func__,"block=%d, location=%d: read failed\n" , + block, location);return NULL ; }
@@ -221,22 +222,27 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_debug ("location mismatch block %u, tag %u != %u\n" , - block,le32_to_cpu (tag_p->tagLocation), location); +udf_err (sb, __func__, +"location mismatch block %u, tag %u != %u\n" , + block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +udf_err (sb, __func__, +"tag checksum failed block %d, checksum 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , -le16_to_cpu (tag_p->descVersion), block); +udf_err (sb, __func__, +"tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , +le16_to_cpu (tag_p->descVersion), block); goto error_out; }
@@ -247,8 +253,10 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_debug ("Crc failure block %d: crc = %d, crclen = %d\n" , block, -le16_to_cpu (tag_p->descCRC),le16_to_cpu (tag_p->descCRCLength)); +udf_err (sb, __func__, +"Crc failure block %d: crc = %d, crclen = %d\n" , + block,le16_to_cpu (tag_p->descCRC), +le16_to_cpu (tag_p->descCRCLength)); error_out:brelse (bh);
fs/udf/namei.c +8 -7
@@ -799,9 +799,9 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)if (retval) goto end_rmdir;if (inode->i_nlink !=2 ) -udf_warning (inode->i_sb,"udf_rmdir" , -"empty directory has nlink != 2 (%d)" , - inode->i_nlink); +udf_warn (inode->i_sb, __func__, +"empty directory has nlink != 2 (%d)\n" , + inode->i_nlink);clear_nlink (inode); inode->i_size =0 ;inode_dec_link_count (dir);
@@ -838,8 +838,9 @@ static int udf_unlink(struct inode *dir, struct dentry *dentry) goto end_unlink;if (!inode->i_nlink) { -udf_debug ("Deleting nonexistent file (%lu), %d\n" , - inode->i_ino, inode->i_nlink); +udf_debug (dir->i_sb, __func__, +"Deleting nonexistent file (%lu), %d\n" , + inode->i_ino, inode->i_nlink); inode->i_nlink =1 ; } retval =udf_delete_entry (dir, fi, &fibh, &cfi);
@@ -1111,7 +1112,7 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry, } retval = -EIO ;if (old_iinfo->i_alloc_type ==ICBTAG_FLAG_AD_IN_ICB ) { - dir_fi = udf_get_fileident( + dir_fi = udf_get_fileident(old_inode, old_iinfo->i_ext.i_data - (old_iinfo->i_efe ? sizeof(struct extendedFileEntry) :
@@ -1121,7 +1122,7 @@ static int udf_rename(struct inode *old_dir, struct dentry *old_dentry, dir_bh =udf_bread (old_inode,0 ,0 , &retval);if (!dir_bh) goto end_rename; - dir_fi =udf_get_fileident (dir_bh->b_data, + dir_fi =udf_get_fileident (old_inode, dir_bh->b_data, old_inode->i_sb->s_blocksize, &offset); }if (!dir_fi)
fs/udf/partition.c +13 -10
@@ -33,8 +33,9 @@ uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,struct udf_sb_info *sbi =UDF_SB (sb);struct udf_part_map *map;if (partition >= sbi->s_partitions) { -udf_debug ("block=%d, partition=%d, offset=%d: " -"invalid partition\n" , block, partition, offset); +udf_debug (sb, __func__, +"block=%d, partition=%d, offset=%d: invalid partition\n" , + block, partition, offset);return 0xFFFFFFFF ; } map = &sbi->s_partmaps[partition];
@@ -60,8 +61,9 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block, vdata = &map->s_type_specific.s_virtual;if (block > vdata->s_num_entries) { -udf_debug ("Trying to access block beyond end of VAT " -"(%d max %d)\n" , block, vdata->s_num_entries); +udf_debug (sb, __func__, +"Trying to access block beyond end of VAT (%d max %d)\n" , + block, vdata->s_num_entries);return 0xFFFFFFFF ; }
@@ -84,8 +86,9 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block, bh =sb_bread (sb, loc);if (!bh) { -udf_debug ("get_pblock(UDF_VIRTUAL_MAP:%p,%d,%d) VAT: %d[%d]\n" , - sb, block, partition, loc, index); +udf_debug (sb, __func__, +"get_pblock(UDF_VIRTUAL_MAP:%p,%d,%d) VAT: %d[%d]\n" , + sb, block, partition, loc, index);return 0xFFFFFFFF ; }
@@ -95,7 +98,7 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block, translate:if (iinfo->i_location.partitionReferenceNum == partition) { -udf_debug ("recursive call to udf_get_pblock!\n" ); +udf_debug (sb, __func__,"recursive call to udf_get_pblock!\n" );return 0xFFFFFFFF ; }
@@ -312,7 +315,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,uint32_t retblk;struct inode *inode; -udf_debug ("READING from METADATA\n" ); +udf_debug (sb, __func__,"READING from METADATA\n" ); map = &sbi->s_partmaps[partition]; mdata = &map->s_type_specific.s_metadata;
@@ -322,8 +325,8 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,BUG_ON (!inode); retblk =udf_try_read_meta (inode, block, partition, offset);if (retblk ==0xFFFFFFFF ) { -udf_warning (sb, __func__,"error reading from METADATA, " -"trying to read from MIRROR" ); +udf_warn (sb, __func__, +"error reading from METADATA, trying to read from MIRROR" ); inode = mdata->s_mirror_fe;if (!inode)return 0xFFFFFFFF ;
fs/udf/super.c +223 -166
775 lines not on this page · raw message
fs/udf/truncate.c +13 -12
@@ -95,23 +95,24 @@ void udf_truncate_tail_extent(struct inode *inode) lbcount += elen;if (lbcount > inode->i_size) {if (lbcount - inode->i_size >= inode->i_sb->s_blocksize) -printk (KERN_WARNING -"udf_truncate_tail_extent(): Too long " -"extent after EOF in inode %u: i_size: " -"%Ld lbcount: %Ld extent %u+%u\n" , - (unsigned )inode->i_ino, - (long long )inode->i_size, - (long long )lbcount, - (unsigned )eloc.logicalBlockNum, - (unsigned )elen); +udf_warn (inode->i_sb, __func__, +"udf_truncate_tail_extent(): Too long " +"extent after EOF in inode %u: i_size: " +"%lld lbcount: %lld extent %u+%u\n" , + (unsigned )inode->i_ino, + (long long )inode->i_size, + (long long )lbcount, + (unsigned )eloc.logicalBlockNum, + (unsigned )elen); nelen = elen - (lbcount - inode->i_size); epos.offset -= adsize;extent_trunc (inode, &epos, &eloc, etype, elen, nelen); epos.offset += adsize;if (udf_next_aext (inode, &epos, &eloc, &elen,1 ) !=-1 ) -printk (KERN_ERR "udf_truncate_tail_extent(): " -"Extent after EOF in inode %u.\n" , - (unsigned )inode->i_ino); +udf_warn (inode->i_sb, __func__, +"udf_truncate_tail_extent(): " +"Extent after EOF in inode %u.\n" , + (unsigned )inode->i_ino);break ; } }
fs/udf/udfdecl.h +23 -20
76 lines not on this page · raw message
fs/udf/udftime.c +1 -1
@@ -34,9 +34,9 @@* http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm */ +#include "udfdecl.h" #include <linux/types.h> #include <linux/kernel.h> -#include "udfdecl.h" #define EPOCH_YEAR 1970
fs/udf/unicode.c +23 -19
103 lines not on this page · raw message
Signature · 1 line
1.7.4.4
cover udf: Change some KERN_<level>s and logging neatening
I reviewed your patch and think it's too large for others to review because it combines multiple things into a single patch.
I think that adding struct super_block * to udb_debug isn't necessary or particularly useful.
Here are the changes to your original patch that I made:
Separate a single large patch into reviewable chunks. Don't convert every message type to udf_<foo>.
Joe Perches (6):
udf: Promote some debugging messages to udf_error
udf: Rename udf_error to udf_err
udf: Rename udf_warn to udf_warn
udf: Convert printks to pr_<level>
udf: Neaten logging output, use vsprintf extension %pV
udf: Neaten udf_debug uses
fs/udf/balloc.c | 14 ++-- fs/udf/directory.c | 8 +- fs/udf/inode.c | 36 ++++----- fs/udf/lowlevel.c | 2 +- fs/udf/misc.c | 24 ++++--- fs/udf/namei.c | 5 +- fs/udf/partition.c | 11 +-- fs/udf/super.c | 214 +++++++++++++++++++++++----------------------------- fs/udf/truncate.c | 21 ++--- fs/udf/udfdecl.h | 35 ++++++--- fs/udf/udftime.c | 3 +- fs/udf/unicode.c | 6 +- 12 files changed, 178 insertions(+), 201 deletions(-)
Signature · 1 line
1.7.6.405.gc1be0
PATCH 1/6 udf: Promote some debugging messages to udf_error
If there is a problem with a scratched disc or loader, it's valuable to know which error occurred.
Convert some debug messages to udf_error, neaten those messages too. Add the calculated tag checksum and the read checksum to error message. Make udf_error a public function and move the logging prototypes together.
--- fs/udf/misc.c | 21 ++++++++++++++------- fs/udf/super.c | 6 ++---- fs/udf/udfdecl.h | 9 +++++++-- 3 files changed, 23 insertions(+), 13 deletions(-)
fs/udf/misc.c +14 -7
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )
@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , +udf_error (sb, __func__,"read failed, block=%u, location=%d\n" , block, location);return NULL ; }
@@ -221,21 +222,26 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_debug ("location mismatch block %u, tag %u != %u\n" , +udf_error (sb, __func__, +"location mismatch, block %u, tag %u != %u\n" , block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +udf_error (sb, __func__, +"tag checksum failed, block %u: 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , +udf_error (sb, __func__, +"tag version 0x%04x != 0x0002 || 0x0003, block %u\n" ,le16_to_cpu (tag_p->descVersion), block); goto error_out; }
@@ -247,8 +253,9 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_debug ("Crc failure block %d: crc = %d, crclen = %d\n" , block, -le16_to_cpu (tag_p->descCRC),le16_to_cpu (tag_p->descCRCLength)); +udf_error (sb, __func__,"crc failure, block %u: crc = %d, crclen = %d\n" , + block,le16_to_cpu (tag_p->descCRC), +le16_to_cpu (tag_p->descCRCLength)); error_out:brelse (bh);
fs/udf/super.c +2 -4
@@ -92,8 +92,6 @@ static void udf_close_lvid(struct super_block *);static unsigned int udf_count_free (struct super_block *);static int udf_statfs (struct dentry *,struct kstatfs *);static int udf_show_options (struct seq_file *,struct vfsmount *); -static void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...);struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu (struct udf_sb_info *sbi) {
@@ -2096,8 +2094,8 @@ error_out:return -EINVAL ; } -static void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...) +void udf_error (struct super_block *sb,const char *function, +const char *fmt, ...) {va_list args;
fs/udf/udfdecl.h +7 -2
@@ -29,6 +29,13 @@ do { \ #define udf_debug(f, a...) /**/#endif +__attribute__((format (printf,3 ,4 ))) +extern void udf_warning (struct super_block *,const char *,const char *, ...); + +__attribute__((format (printf,3 ,4 ))) +extern void udf_error (struct super_block *sb,const char *function, +const char *fmt, ...); + #define udf_info(f, a...) \ printk(KERN_INFO "UDF-fs INFO " f, ##a);
@@ -112,8 +119,6 @@ struct extent_position {/* super.c */ -__attribute__((format (printf,3 ,4 ))) -extern void udf_warning (struct super_block *,const char *,const char *, ...);static inline void udf_updated_lvid (struct super_block *sb) {struct buffer_head *bh =UDF_SB (sb)->s_lvid_bh;
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 1/6] udf: Promote some debugging messages to udf_error
2011/10/10 Joe Perches <[email protected]>:
Quoted 9 lines
If there is a problem with a scratched disc or loader, it's valuable to know which error occurred.
Convert some debug messages to
udf_error, neaten those messages too. Add the calculated tag checksum and the read checksum to error message. Makeudf_errora public function and move the logging prototypes together.Original-patch-by NamJae Jeon <[email protected]>Signed-off-by Joe Perches <[email protected]>
Quoted 125 lines
---
fs/udf/misc.c | 21 ++++++++++++++-------
fs/udf/super.c | 6 ++----
fs/udf/udfdecl.h | 9 +++++++--3 files changed, 23 insertions(+), 13 deletions(-)fs/udf/misc.c +9
-4@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, { struct tag *tag_p; struct buffer_head *bh = NULL; + u8 checksum; /* Read the block */ if (block == 0xFFFFFFFF)@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh = udf_tread(sb, block); if (!bh) { - udf_debug("block=%d, location=%d: read failed\n", + udf_error(sb, __func__, "read failed, block=%u, location=%d\n", block, location); return NULL; }@@ -221,21 +222,26 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident = le16_to_cpu(tag_p->tagIdent); if (location != le32_to_cpu(tag_p->tagLocation)) { - udf_debug("location mismatch block %u, tag %u != %u\n", + udf_error(sb, __func__, + "location mismatch, block %u, tag %u != %u\n", block, le32_to_cpu(tag_p->tagLocation), location); goto error_out; } /* Verify the tag checksum */ - if (udf_tag_checksum(tag_p) != tag_p->tagChecksum) { - printk(KERN_ERR "udf: tag checksum failed block %d\n", block); + checksum = udf_tag_checksum(tag_p); + if (checksum != tag_p->tagChecksum) { + udf_error(sb, __func__, + "tag checksum failed, block %u: 0x%02x != 0x%02x\n", + block, checksum, tag_p->tagChecksum); goto error_out; } /* Verify the tag version */if (
tag_p->descVersion!=cpu_to_le16(0x0002U)&&tag_p->descVersion!=cpu_to_le16(0x0003U)) {
- udf_debug("tag version 0x%04x !=0x0002||0x0003block %d\n",
+ udf_error(sb,__func__,
+ "tag version 0x%04x !=0x0002||0x0003, block %u\n",le16_to_cpu(tag_p->descVersion), block); gotoerror_out;
}fs/udf/misc.c +3
-2@@ -247,8 +253,9 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength))) return bh; - udf_debug("Crc failure block %d: crc = %d, crclen = %d\n", block, - le16_to_cpu(tag_p->descCRC), le16_to_cpu(tag_p->descCRCLength)); + udf_error(sb, __func__, "crc failure, block %u: crc = %d, crclen = %d\n", + block, le16_to_cpu(tag_p->descCRC), + le16_to_cpu(tag_p->descCRCLength));error_out:
brelse(bh);fs/udf/super.c +2
-4@@ -92,8 +92,6 @@ static void udf_close_lvid(struct super_block *); static unsigned int udf_count_free(struct super_block *); static int udf_statfs(struct dentry *, struct kstatfs *); static int udf_show_options(struct seq_file *, struct vfsmount *); -static void udf_error (struct super_block *sb,const char *function, - const char *fmt, ...); struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu(struct udf_sb_info *sbi) {@@ -2096,8 +2094,8 @@ error_out: return -EINVAL; } -static void udf_error (struct super_block *sb,const char *function, - const char *fmt, ...) +void udf_error (struct super_block *sb,const char *function, + const char *fmt, ...) { va_list args;fs/udf/udfdecl.h +7
-3@@ -29,6 +29,13 @@ do { \ #define udf_debug(f, a...) /**/ #endif +__attribute__((format (printf,3 ,4 ))) +extern void udf_warning (struct super_block *,const char *,const char *, ...); + +__attribute__((format (printf,3 ,4 ))) +extern void udf_error (struct super_block *sb,const char *function, + const char *fmt, ...); + #define udf_info(f, a...) \ printk(KERN_INFO "UDF-fs INFO " f, ##a);@@ -112,8 +119,6 @@ struct extent_position { /* super.c */ -__attribute__((format (printf,3 ,4 ))) -extern void udf_warning (struct super_block *,const char *,const char *, ...); static inline void udf_updated_lvid(struct super_block *sb) { struct buffer_head *bh = UDF_SB(sb)->s_lvid_bh; --1.7.6.405.gc1be0
Re: [PATCH 1/6] udf: Promote some debugging messages to udf_error
In reply to Joe PerchesQuoted 9 lines · Joe Perches
If there is a problem with a scratched disc or loader, it's valuable to know which error occurred.
Convert some debug messages to
udf_error, neaten those messages too. Add the calculated tag checksum and the read checksum to error message. Makeudf_errora public function and move the logging prototypes together.Original-patch-by NamJae Jeon <[email protected]>Signed-off-by Joe Perches <[email protected]>
I have now tested this patch and found out it's not such a good idea as
it seems at the first sight. UDF uses udf_read_tagged() to probe whether some structure is in some location (because some structures can be placed at several places in the filesystem and the only way to determine where the structure is to load blocks from all the places and check block headers). So as a result of this patch normal UDF mount produces about 20 error messages... So I've altered this patch to change only some messages in udf_read_tagged(). The result is below.
Honza
Signature · 3 lines
Jan Kara <[email protected]> SUSE Labs, CR ---
Quoted 1 line
From
7e273e3b41e32716dc122b293b5f15635af495ffMon Sep 17 00:00:00 2001
From: Joe Perches <[email protected]>
Date: Mon, 10 Oct 2011 01:08:02 -0700
Subject: [PATCH] udf: Promote some debugging messages to udf_error
If there is a problem with a scratched disc or loader, it's valuable to know which error occurred.
Convert some debug messages to udf_error, neaten those messages too. Add the calculated tag checksum and the read checksum to error message. Make udf_error a public function and move the logging prototypes together.
--- fs/udf/misc.c | 13 +++++++++---- fs/udf/super.c | 6 ++---- fs/udf/udfdecl.h | 9 +++++++-- 3 files changed, 18 insertions(+), 10 deletions(-)
fs/udf/misc.c +9 -4
@@ -204,6 +204,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, {struct tag *tag_p;struct buffer_head *bh =NULL ; +u8 checksum;/* Read the block */ if (block ==0xFFFFFFFF )
@@ -211,7 +212,7 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_debug ("block=%d, location=%d: read failed\n" , +udf_error (sb, __func__,"read failed, block=%u, location=%d\n" , block, location);return NULL ; }
@@ -227,15 +228,19 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, }/* Verify the tag checksum */ -if (udf_tag_checksum (tag_p) != tag_p->tagChecksum) { -printk (KERN_ERR "udf: tag checksum failed block %d\n" , block); + checksum =udf_tag_checksum (tag_p); +if (checksum != tag_p->tagChecksum) { +udf_error (sb, __func__, +"tag checksum failed, block %u: 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_debug ("tag version 0x%04x != 0x0002 || 0x0003 block %d\n" , +udf_error (sb, __func__, +"tag version 0x%04x != 0x0002 || 0x0003, block %u\n" ,le16_to_cpu (tag_p->descVersion), block); goto error_out; }
fs/udf/super.c +2 -4
@@ -92,8 +92,6 @@ static void udf_close_lvid(struct super_block *);static unsigned int udf_count_free (struct super_block *);static int udf_statfs (struct dentry *,struct kstatfs *);static int udf_show_options (struct seq_file *,struct vfsmount *); -static void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...);struct logicalVolIntegrityDescImpUse *udf_sb_lvidiu (struct udf_sb_info *sbi) {
@@ -2096,8 +2094,8 @@ error_out:return -EINVAL ; } -static void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...) +void udf_error (struct super_block *sb,const char *function, +const char *fmt, ...) {va_list args;
fs/udf/udfdecl.h +7 -2
@@ -29,6 +29,13 @@ do { \ #define udf_debug(f, a...) /**/#endif +__attribute__((format (printf,3 ,4 ))) +extern void udf_warning (struct super_block *,const char *,const char *, ...); + +__attribute__((format (printf,3 ,4 ))) +extern void udf_error (struct super_block *sb,const char *function, +const char *fmt, ...); + #define udf_info(f, a...) \ printk(KERN_INFO "UDF-fs INFO " f, ##a);
@@ -112,8 +119,6 @@ struct extent_position {/* super.c */ -__attribute__((format (printf,3 ,4 ))) -extern void udf_warning (struct super_block *,const char *,const char *, ...);static inline void udf_updated_lvid (struct super_block *sb) {struct buffer_head *bh =UDF_SB (sb)->s_lvid_bh;
Signature · 1 line
1.7.1
PATCH 2/6 udf: Rename udf_error to udf_err
In reply to Joe PerchesRename udf_error to udf_err for consistency with normal logging uses of pr_err.
Rename function udf_err to _udf_err.
Remove __func__ from uses and move __func__ to a new udf_err macro that calls _udf_err.
Some of the udf_error uses had \n terminations, some did not so standardize \n's to udf_err uses, remove \n from _udf_err function. Coalesce udf_err formats.
One message prefixed with udf_read_super is now prefixed with udf_fill_super.
--- fs/udf/misc.c | 25 +++++++++++-------------- fs/udf/super.c | 27 +++++++++++---------------- fs/udf/udfdecl.h | 7 ++++--- 3 files changed, 26 insertions(+), 33 deletions(-)
fs/udf/misc.c +11 -14
@@ -212,8 +212,8 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, bh =udf_tread (sb, block);if (!bh) { -udf_error (sb, __func__,"read failed, block=%u, location=%d\n" , - block, location); +udf_err (sb,"read failed, block=%u, location=%d\n" , + block, location);return NULL ; }
@@ -222,27 +222,24 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, *ident =le16_to_cpu (tag_p->tagIdent);if (location !=le32_to_cpu (tag_p->tagLocation)) { -udf_error (sb, __func__, -"location mismatch, block %u, tag %u != %u\n" , - block,le32_to_cpu (tag_p->tagLocation), location); +udf_err (sb,"location mismatch, block %u, tag %u != %u\n" , + block,le32_to_cpu (tag_p->tagLocation), location); goto error_out; }/* Verify the tag checksum */ checksum =udf_tag_checksum (tag_p);if (checksum != tag_p->tagChecksum) { -udf_error (sb, __func__, -"tag checksum failed, block %u: 0x%02x != 0x%02x\n" , - block, checksum, tag_p->tagChecksum); +udf_err (sb,"tag checksum failed, block %u: 0x%02x != 0x%02x\n" , + block, checksum, tag_p->tagChecksum); goto error_out; }/* Verify the tag version */ if (tag_p->descVersion !=cpu_to_le16 (0x0002U ) && tag_p->descVersion !=cpu_to_le16 (0x0003U )) { -udf_error (sb, __func__, -"tag version 0x%04x != 0x0002 || 0x0003, block %u\n" , -le16_to_cpu (tag_p->descVersion), block); +udf_err (sb,"tag version 0x%04x != 0x0002 || 0x0003, block %u\n" , +le16_to_cpu (tag_p->descVersion), block); goto error_out; }
@@ -253,9 +250,9 @@ struct buffer_head *udf_read_tagged(struct super_block *sb, uint32_t block, le16_to_cpu(tag_p->descCRCLength)))return bh; -udf_error (sb, __func__,"crc failure, block %u: crc = %d, crclen = %d\n" , - block,le16_to_cpu (tag_p->descCRC), -le16_to_cpu (tag_p->descCRCLength)); +udf_err (sb,"crc failure, block %u: crc = %d, crclen = %d\n" , + block,le16_to_cpu (tag_p->descCRC), +le16_to_cpu (tag_p->descCRCLength)); error_out:brelse (bh);
fs/udf/super.c +11 -16
@@ -242,9 +242,8 @@ static int udf_sb_alloc_partition_maps(struct super_block *sb, u32 count) sbi->s_partmaps =kcalloc (count,sizeof (struct udf_part_map ),GFP_KERNEL );if (!sbi->s_partmaps) { -udf_error (sb, __func__, -"Unable to allocate space for %d partition maps" , - count); +udf_err (sb,"Unable to allocate space for %d partition maps\n" , + count); sbi->s_partitions =0 ;return -ENOMEM ; }
@@ -879,8 +878,7 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)if (mdata->s_mirror_fe ==NULL ) {if (fe_error) { -udf_error (sb, __func__,"mirror inode efe not found " -"and metadata inode is missing too, exiting..." ); +udf_err (sb,"mirror inode efe not found and metadata inode is missing too, exiting...\n" ); goto error_exit; }else udf_warning(sb, __func__, "mirror inode efe not found,"
@@ -915,8 +913,7 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)"not found but it's ok since the disc" " is mounted read-only"); else { -udf_error (sb, __func__,"bitmap inode efe not " -"found and attempted read-write mount" ); +udf_err (sb,"bitmap inode efe not found and attempted read-write mount\n" ); goto error_exit; } }
@@ -969,9 +966,8 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index) bitmap =vzalloc (size);/* TODO: get rid of vzalloc */ if (bitmap ==NULL ) { -udf_error (sb, __func__, -"Unable to allocate space for bitmap " -"and %d buffer_head pointers" , nr_groups); +udf_err (sb,"Unable to allocate space for bitmap and %d buffer_head pointers\n" , + nr_groups);return NULL ; }
@@ -1935,8 +1931,7 @@ static int udf_fill_super(struct super_block *sb, void *options, int silent)if (uopt.flags & (1 <<UDF_FLAG_UTF8 ) && uopt.flags & (1 <<UDF_FLAG_NLS_MAP )) { -udf_error (sb,"udf_read_super" , -"utf8 cannot be combined with iocharset\n" ); +udf_err (sb,"utf8 cannot be combined with iocharset\n" ); goto error_out; }#ifdef CONFIG_UDF_NLS
@@ -2094,8 +2089,8 @@ error_out:return -EINVAL ; } -void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...) +void _udf_err (struct super_block *sb,const char *function, +const char *fmt, ...) {va_list args;
@@ -2106,8 +2101,8 @@ void udf_error(struct super_block *sb, const char *function,va_start (args, fmt);vsnprintf (error_buf,sizeof (error_buf), fmt, args);va_end (args); -printk (KERN_CRIT "UDF-fs error (device %s): %s: %s\n" , - sb->s_id, function, error_buf); +printk (KERN_CRIT "UDF-fs error (device %s): %s: %s" , + sb->s_id, function, error_buf); } void udf_warning(struct super_block *sb, const char *function,
fs/udf/udfdecl.h +4 -3
@@ -33,13 +33,14 @@ __attribute__((format(printf, 3, 4)))extern void udf_warning (struct super_block *,const char *,const char *, ...); __attribute__((format (printf,3 ,4 ))) -extern void udf_error (struct super_block *sb,const char *function, -const char *fmt, ...); +extern void _udf_err (struct super_block *sb,const char *function, +const char *fmt, ...); +#define udf_err (sb, fmt, ...) \ + _udf_err(sb, __func__, fmt, ##__VA_ARGS__) #define udf_info(f, a...) \ printk(KERN_INFO "UDF-fs INFO " f, ##a); -#define udf_fixed_to_variable (x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )#define udf_variable_to_fixed (x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 2/6] udf: Rename udf_error to udf_err
2011/10/10 Joe Perches <[email protected]>:
Quoted 12 lines
Rename
udf_errortoudf_errfor consistency with normal logging uses ofpr_err.Rename function
udf_errto_udf_err.
Remove__func__from uses and move__func__to a newudf_errmacro that calls_udf_err.
Some of theudf_erroruses had \n terminations, some did not so standardize \n's toudf_erruses, remove \n from_udf_errfunction. Coalesceudf_errformats.
One message prefixed withudf_read_superis now prefixed withudf_fill_super.
Quoted 1 line
Signed-off-by Joe Perches <[email protected]>
PATCH 3/6 udf: Rename udf_warn to udf_warn
In reply to Joe PerchesRename udf_warning to udf_warn for consistency with normal logging uses of pr_warn.
Rename function udf_warning to _udf_warn.
Remove __func__ from uses and move __func__ to a new udf_warn macro that calls _udf_warn.
Add \n's to uses of udf_warn, remove \n from _udf_warn. Coalesce formats.
--- fs/udf/namei.c | 5 ++--- fs/udf/partition.c | 3 +-- fs/udf/super.c | 22 ++++++++-------------- fs/udf/udfdecl.h | 5 ++++- 4 files changed, 15 insertions(+), 20 deletions(-)
fs/udf/namei.c +2 -3
@@ -799,9 +799,8 @@ static int udf_rmdir(struct inode *dir, struct dentry *dentry)if (retval) goto end_rmdir;if (inode->i_nlink !=2 ) -udf_warning (inode->i_sb,"udf_rmdir" , -"empty directory has nlink != 2 (%d)" , - inode->i_nlink); +udf_warn (inode->i_sb,"empty directory has nlink != 2 (%d)\n" , + inode->i_nlink);clear_nlink (inode); inode->i_size =0 ;inode_dec_link_count (dir);
fs/udf/partition.c +1 -2
@@ -322,8 +322,7 @@ uint32_t udf_get_pblock_meta25(struct super_block *sb, uint32_t block,BUG_ON (!inode); retblk =udf_try_read_meta (inode, block, partition, offset);if (retblk ==0xFFFFFFFF ) { -udf_warning (sb, __func__,"error reading from METADATA, " -"trying to read from MIRROR" ); +udf_warn (sb,"error reading from METADATA, trying to read from MIRROR\n" ); inode = mdata->s_mirror_fe;if (!inode)return 0xFFFFFFFF ;
fs/udf/super.c +8 -14
@@ -855,13 +855,11 @@ static int udf_load_metadata_files(struct super_block *sb, int partition) mdata->s_metadata_fe =udf_iget (sb, &addr);if (mdata->s_metadata_fe ==NULL ) { -udf_warning (sb, __func__,"metadata inode efe not found, " -"will try mirror inode." ); +udf_warn (sb,"metadata inode efe not found, will try mirror inode\n" ); fe_error =1 ; }else if (UDF_I (mdata->s_metadata_fe)->i_alloc_type !=ICBTAG_FLAG_AD_SHORT ) { -udf_warning (sb, __func__,"metadata inode efe does not have " -"short allocation descriptors!" ); +udf_warn (sb,"metadata inode efe does not have short allocation descriptors!\n" ); fe_error =1 ;iput (mdata->s_metadata_fe); mdata->s_metadata_fe =NULL ;
@@ -881,12 +879,10 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)udf_err (sb,"mirror inode efe not found and metadata inode is missing too, exiting...\n" ); goto error_exit; }else -udf_warning (sb, __func__,"mirror inode efe not found," -" but metadata inode is OK" ); +udf_warn (sb,"mirror inode efe not found, but metadata inode is OK\n" ); }else if (UDF_I (mdata->s_mirror_fe)->i_alloc_type !=ICBTAG_FLAG_AD_SHORT ) { -udf_warning (sb, __func__,"mirror inode efe does not have " -"short allocation descriptors!" ); +udf_warn (sb,"mirror inode efe does not have short allocation descriptors!\n" );iput (mdata->s_mirror_fe); mdata->s_mirror_fe =NULL ;if (fe_error)
@@ -909,9 +905,7 @@ static int udf_load_metadata_files(struct super_block *sb, int partition)if (mdata->s_bitmap_fe ==NULL ) {if (sb->s_flags &MS_RDONLY ) -udf_warning (sb, __func__,"bitmap inode efe " -"not found but it's ok since the disc" -" is mounted read-only" ); +udf_warn (sb,"bitmap inode efe not found but it's ok since the disc is mounted read-only\n" );else {udf_err (sb,"bitmap inode efe not found and attempted read-write mount\n" ); goto error_exit;
@@ -2105,15 +2099,15 @@ void _udf_err(struct super_block *sb, const char *function, sb->s_id, function, error_buf); } -void udf_warning (struct super_block *sb,const char *function, -const char *fmt, ...) +void _udf_warn (struct super_block *sb,const char *function, +const char *fmt, ...) {va_list args;va_start (args, fmt);vsnprintf (error_buf,sizeof (error_buf), fmt, args);va_end (args); -printk (KERN_WARNING "UDF-fs warning (device %s): %s: %s\n" , +printk (KERN_WARNING "UDF-fs warning (device %s): %s: %s" , sb->s_id, function, error_buf); }
fs/udf/udfdecl.h +4 -1
@@ -30,7 +30,10 @@ do { \#endif __attribute__((format (printf,3 ,4 ))) -extern void udf_warning (struct super_block *,const char *,const char *, ...); +extern void _udf_warn (struct super_block *sb,const char *function, +const char *fmt, ...); +#define udf_warn (sb, fmt, ...) \ + _udf_warn(sb, __func__, fmt, ##__VA_ARGS__) __attribute__((format(printf, 3, 4))) extern void _udf_err(struct super_block *sb, const char *function,
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 3/6] udf: Rename udf_warn to udf_warn
2011/10/10 Joe Perches <[email protected]>:
Quoted 10 lines
Rename
udf_warningtoudf_warnfor consistency with normal logging uses ofpr_warn.Rename function
udf_warningto_udf_warn.
Remove__func__from uses and move__func__to a newudf_warnmacro that calls_udf_warn.
Add \n's to uses ofudf_warn, remove \n from_udf_warn. Coalesce formats.Signed-off-by Joe Perches <[email protected]>
Quoted 1 line
---
Re: [PATCH 3/6] udf: Rename udf_warn to udf_warn
In reply to Joe PerchesQuoted 2 lines · Joe Perches
Rename
udf_warningtoudf_warnfor consistency with normal logging uses ofpr_warn.
So the summary should read:
udf: Rename udf_warning to udf_warn
Paul Bolle
Re: [PATCH 3/6] udf: Rename udf_warn to udf_warn
Quoted 6 lines · Paul Bolle
Quoted 2 lines · Joe Perches
Rename
udf_warningtoudf_warnfor consistency with normal logging uses ofpr_warn.So the summary should read:
udf: Renameudf_warningtoudf_warn
Yeah, I stuffed that subject line up.
Jan/Honza caught it and fixed it though.
PATCH 4/6 udf: Convert printks to pr_<level>
In reply to Joe PerchesUse the current logging styles.
Convert a few printks that should have been udf_warn and udf_err. Coalesce formats. Add #define pr_fmt.
Move an #include "udfdecls.h" above other includes in udftime.c so pr_fmt works correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions in udfdecl.h
--- fs/udf/directory.c | 4 +- fs/udf/inode.c | 33 +++++++++++-------------- fs/udf/super.c | 67 +++++++++++++++++++++------------------------------ fs/udf/truncate.c | 21 +++++++--------- fs/udf/udfdecl.h | 38 ++++++++++++++-------------- fs/udf/udftime.c | 3 +- fs/udf/unicode.c | 6 ++-- 7 files changed, 77 insertions(+), 95 deletions(-)
fs/udf/directory.c +2 -2
@@ -201,7 +201,7 @@ struct short_ad *udf_get_fileshortad(uint8_t *ptr, int maxoffset, uint32_t *offsstruct short_ad *sa;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_fileshortad() invalidparms\n" ); +pr_err ("%s: invalidparms\n" , __func__);return NULL ; }
@@ -223,7 +223,7 @@ struct long_ad *udf_get_filelongad(uint8_t *ptr, int maxoffset, uint32_t *offsetstruct long_ad *la;if ((!ptr) || (!offset)) { -printk (KERN_ERR "udf: udf_get_filelongad() invalidparms\n" ); +pr_err ("%s: invalidparms\n" , __func__);return NULL ; }
fs/udf/inode.c +14 -19
@@ -83,12 +83,10 @@ void udf_evict_inode(struct inode *inode)end_writeback (inode);if (iinfo->i_alloc_type !=ICBTAG_FLAG_AD_IN_ICB && inode->i_size != iinfo->i_lenExtents) { -printk (KERN_WARNING "UDF-fs (%s): Inode %lu (mode %o) has " -"inode size %llu different from extent length %llu. " -"Filesystem need not be standards compliant.\n" , - inode->i_sb->s_id, inode->i_ino, inode->i_mode, - (unsigned long long )inode->i_size, - (unsigned long long )iinfo->i_lenExtents); +udf_warn (inode->i_sb,"Inode %lu (mode %o) has inode size %llu different from extent length %llu. Filesystem need not be standards compliant.\n" , + inode->i_ino, inode->i_mode, + (unsigned long long )inode->i_size, + (unsigned long long )iinfo->i_lenExtents); }kfree (iinfo->i_ext.i_data); iinfo->i_ext.i_data =NULL ;
@@ -1169,16 +1167,15 @@ static void __udf_read_inode(struct inode *inode)*/ bh =udf_read_ptagged (inode->i_sb, &iinfo->i_location,0 , &ident);if (!bh) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) failed !bh\n" , - inode->i_ino); +udf_err (inode->i_sb,"(ino %ld) failed !bh\n" , inode->i_ino);make_bad_inode (inode);return ; }if (ident !=TAG_IDENT_FE && ident !=TAG_IDENT_EFE && ident !=TAG_IDENT_USE ) { -printk (KERN_ERR "udf: udf_read_inode(ino %ld) " -"failed ident=%d\n" , inode->i_ino, ident); +udf_err (inode->i_sb,"(ino %ld) failed ident=%d\n" , + inode->i_ino, ident);brelse (bh);make_bad_inode (inode);return ;
@@ -1218,7 +1215,7 @@ static void __udf_read_inode(struct inode *inode) }brelse (ibh); }else if (fe->icbTag.strategyType !=cpu_to_le16 (4 )) { -printk (KERN_ERR "udf: unsupported strategy type: %d\n" , +pr_err ("unsupported strategy type: %d\n" ,le16_to_cpu (fe->icbTag.strategyType));brelse (bh);make_bad_inode (inode);
@@ -1413,9 +1410,8 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)udf_debug ("METADATA BITMAP FILE-----\n" );break ;default : -printk (KERN_ERR "udf: udf_fill_inode(ino %ld) failed unknown " -"file type=%d\n" , inode->i_ino, - fe->icbTag.fileType); +udf_err (inode->i_sb,"(ino %ld) failed unknown file type=%d\n" , + inode->i_ino, fe->icbTag.fileType);make_bad_inode (inode);return ; }
@@ -1438,8 +1434,8 @@ static int udf_alloc_i_data(struct inode *inode, size_t size) iinfo->i_ext.i_data =kmalloc (size,GFP_KERNEL );if (!iinfo->i_ext.i_data) { -printk (KERN_ERR "udf:udf_alloc_i_data (ino %ld) " -"no free memory\n" , inode->i_ino); +udf_err (inode->i_sb,"(ino %ld) no free memory\n" , + inode->i_ino);return -ENOMEM ; }
@@ -1689,9 +1685,8 @@ out:if (do_sync) {sync_dirty_buffer (bh);if (buffer_write_io_error (bh)) { -printk (KERN_WARNING "IO error syncing udf inode " -"[%s:%08lx]\n" , inode->i_sb->s_id, - inode->i_ino); +udf_warn (inode->i_sb,"IO error syncing udf inode [%08lx]\n" , + inode->i_ino); err = -EIO ; } }
fs/udf/super.c +28 -39
192 lines not on this page · raw message
fs/udf/truncate.c +9 -12
@@ -95,23 +95,20 @@ void udf_truncate_tail_extent(struct inode *inode) lbcount += elen;if (lbcount > inode->i_size) {if (lbcount - inode->i_size >= inode->i_sb->s_blocksize) -printk (KERN_WARNING -"udf_truncate_tail_extent(): Too long " -"extent after EOF in inode %u: i_size: " -"%Ld lbcount: %Ld extent %u+%u\n" , - (unsigned )inode->i_ino, - (long long )inode->i_size, - (long long )lbcount, - (unsigned )eloc.logicalBlockNum, - (unsigned )elen); +pr_warn ("%s: Too long extent after EOF in inode %u: i_size: %lld lbcount: %lld extent %u+%u\n" , + __func__, + (unsigned )inode->i_ino, + (long long )inode->i_size, + (long long )lbcount, + (unsigned )eloc.logicalBlockNum, + (unsigned )elen); nelen = elen - (lbcount - inode->i_size); epos.offset -= adsize;extent_trunc (inode, &epos, &eloc, etype, elen, nelen); epos.offset += adsize;if (udf_next_aext (inode, &epos, &eloc, &elen,1 ) !=-1 ) -printk (KERN_ERR "udf_truncate_tail_extent(): " -"Extent after EOF in inode %u.\n" , - (unsigned )inode->i_ino); +pr_err ("%s: Extent after EOF in inode %u\n" , + __func__, (unsigned )inode->i_ino);break ; } }
fs/udf/udfdecl.h +19 -19
@@ -1,6 +1,8 @@#ifndef __UDF_DECL_H#define __UDF_DECL_H +#define pr_fmt (fmt) "UDF-fs: " fmt +#include "ecma_167.h" #include "osta_udf.h"
@@ -16,18 +18,11 @@#define UDF_PREALLOCATE #define UDF_DEFAULT_PREALLOC_BLOCKS 8 -#undef UDFFS_DEBUG - -#ifdef UDFFS_DEBUG -#define udf_debug(f, a...) \ -do { \ - printk(KERN_DEBUG "UDF-fs DEBUG %s:%d:%s: ", \ - __FILE__, __LINE__, __func__); \ - printk(f, ##a); \ -} while (0) -#else -#define udf_debug(f, a...) /**/ -#endif +__attribute__((format (printf,3 ,4 ))) +extern void _udf_err (struct super_block *sb,const char *function, +const char *fmt, ...); +#define udf_err (sb, fmt, ...) \ + _udf_err(sb, __func__, fmt, ##__VA_ARGS__) __attribute__((format(printf, 3, 4))) extern void _udf_warn(struct super_block *sb, const char *function,
@@ -35,14 +30,19 @@ extern void _udf_warn(struct super_block *sb, const char *function,#define udf_warn (sb, fmt, ...) \ _udf_warn(sb, __func__, fmt, ##__VA_ARGS__) -__attribute__((format (printf,3 ,4 ))) -extern void _udf_err (struct super_block *sb,const char *function, -const char *fmt, ...); -#define udf_err (sb, fmt, ...) \ - _udf_err(sb, __func__, fmt, ##__VA_ARGS__) +#define udf_info (fmt, ...) \ + pr_info("INFO " fmt, ##__VA_ARGS__) -#define udf_info(f, a...) \ - printk(KERN_INFO "UDF-fs INFO " f, ##a); +#undef UDFFS_DEBUG + +#ifdef UDFFS_DEBUG +#define udf_debug (fmt, ...) \ + printk(KERN_DEBUG pr_fmt("%s:%d:%s: " fmt), \ + __FILE__, __LINE__, __func__, ##__VA_ARGS_) +#else +#define udf_debug (fmt, ...) \ + no_printk(fmt, ##__VA_ARGS__) +#endif #define udf_fixed_to_variable (x) ( ( ( (x) >> 5 ) * 39 ) + ( (x) & 0x0000001F ) )#define udf_variable_to_fixed (x) ( ( ( (x) / 39 ) << 5 ) + ( (x) % 39 ) )
fs/udf/udftime.c +2 -1
@@ -34,9 +34,10 @@* http://www.boulder.nist.gov/timefreq/pubs/bulletin/leapsecond.htm */ +#include "udfdecl.h" +#include <linux/types.h> #include <linux/kernel.h> -#include "udfdecl.h" #define EPOCH_YEAR 1970
fs/udf/unicode.c +3 -3
@@ -114,7 +114,7 @@ int udf_CS0toUTF8(struct ustr *utf_o, const struct ustr *ocu_i) cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }
@@ -242,7 +242,7 @@ try_again:if (utf_cnt) { error_out: ocu[++u_len] ='?' ; -printk (KERN_DEBUG "udf: bad UTF-8 character\n" ); + printk(KERN_DEBUG pr_fmt("bad UTF-8 character\n")); } ocu[length -1 ] = (uint8_t )u_len +1 ;
@@ -267,7 +267,7 @@ static int udf_CS0toNLS(struct nls_table *nls, struct ustr *utf_o, cmp_id = ocu_i->u_cmpID;if (cmp_id !=8 && cmp_id !=16 ) {memset (utf_o,0 ,sizeof (struct ustr )); -printk (KERN_ERR "udf: unknown compression code (%d) stri=%s\n" , +pr_err ("unknown compression code (%d) stri=%s\n" , cmp_id, ocu_i->u_name);return 0 ; }
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
2011/10/10 Joe Perches <[email protected]>:
Quoted 9 lines
Use the current logging styles.
Convert a few printks that should have been
udf_warnandudf_err. Coalesce formats. Add #definepr_fmt.
Move an #include "udfdecls.h" above other includes inudftime.csopr_fmtworks correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions inudfdecl.hSigned-off-by Joe Perches <[email protected]>
Quoted 1 line
---
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
In reply to Joe PerchesQuoted 7 lines · Joe Perches
Use the current logging styles.
Convert a few printks that should have been
udf_warnandudf_err. Coalesce formats. Add #definepr_fmt.
Move an #include "udfdecls.h" above other includes inudftime.csopr_fmtworks correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions inudfdecl.h
Any reason why you didn't use udf_err() / udf_warn() in all the places
where sb pointer is available? Also I'd further use udf_err() in udf_parse_options() - passing sb there is simple and additional information in the message is useful. Thanks.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
Quoted 9 lines · Jan Kara
Quoted 6 lines · Joe Perches
Use the current logging styles.
Convert a few printks that should have beenudf_warnandudf_err. Coalesce formats. Add #definepr_fmt.
Move an #include "udfdecls.h" above other includes inudftime.csopr_fmtworks correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions inudfdecl.hAny reason why you didn't use
udf_err()/udf_warn()in all the places
where sb pointer is available?
I did it in the places where sb was otherwise used in the output message.
Quoted 3 lines
Also I'd further use
udf_err()inudf_parse_options()- passing sb there is simple and additional information in the message is useful. Thanks.
Maybe NamJae can do that in a follow-on patch.
NamJae, I broke up your larger patch so you could
see an example of the preferred form of smaller,
targeted patches.
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
In reply to Jan KaraQuoted 10 lines · Jan Kara
Quoted 7 lines · Joe Perches
Use the current logging styles.
Convert a few printks that should have been
udf_warnandudf_err. Coalesce formats. Add #definepr_fmt.
Move an #include "udfdecls.h" above other includes inudftime.csopr_fmtworks correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions inudfdecl.hAny reason why you didn't use
udf_err()/udf_warn()in all the places
where sb pointer is available?
Oh, I forgot to mention that udf_err emits at KERN_CRIT. I think that's not good.
It's either an error or it's a critical notification and the pr_crit should be pr_err or the function should be renamed.
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
Quoted 12 lines · Joe Perches
Quoted 10 lines · Jan Kara
Quoted 7 lines · Joe Perches
Use the current logging styles.
Convert a few printks that should have been
udf_warnandudf_err. Coalesce formats. Add #definepr_fmt.
Move an #include "udfdecls.h" above other includes inudftime.csopr_fmtworks correctly. Strip prefixes from conversions as appropriate. Reorder logging definitions inudfdecl.hAny reason why you didn't use
udf_err()/udf_warn()in all the places
where sb pointer is available?
Quoted 6 lines
Oh, I forgot to mention that
udf_erremits atKERN_CRIT. I think that's not good.It's either an error or it's a critical notification and the
pr_critshould bepr_error the function should be renamed.
Yeah, good point. I've changed udf_err() to call pr_err() becauseKERN_CRIT is really too much.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
Quoted 5 lines · Jan Kara
Quoted 2 lines · Joe Perches
I forgot to mention that
udf_erremits atKERN_CRIT. I think that's not good.I've changed
udf_err()to callpr_err()becauseKERN_CRITis really too much.
If this means you applied it or applied it with modifications, I think it'd be good to either reply to the 0/n message with something like "applied to my tree", or reply to each message as appropriate.
That reply lets other people like NamJae proceed appropriately. Otherwise, it's left to some pull request or patch series posted by you potentially months later to figure out what's current.
Is there a udf git tree?
The stuff on sourceforge is not current and the MAINTAINERS W: entry doesn't work.
Maybe:
MAINTAINERS +1 -1
@@ -6624,7 +6624,7 @@ F: drivers/net/ethernet/8390/ne-h8300.c UDF FILESYSTEM M: Jan Kara <[email protected]> -W: http://linux-udf.sourceforge.net +W: http://sourceforge.net/projects/linux-udf/develop S: Maintained F: Documentation/filesystems/udf.txt F: fs/udf/
cheers, Joe
Re: [PATCH 4/6] udf: Convert printks to pr_<level>
Quoted 4 lines · Joe Perches
Quoted 3 lines · Jan Kara
Quoted 2 lines · Joe Perches
I forgot to mention that
udf_erremits atKERN_CRIT. I think that's not good.
Quoted 8 lines
Quoted 2 lines
I've changed
udf_err()to callpr_err()becauseKERN_CRITis really too much.If this means you applied it or applied it with modifications, I think it'd be good to either reply to the 0/n message with something like "applied to my tree", or reply to each message as appropriate.
Sure, that's what I wanted to do but I first wanted to test what I have,
stumbled across an issue with patch 1 which delayed me a bit...
Quoted 7 lines
That reply lets other people like NamJae proceed appropriately. Otherwise, it's left to some pull request or patch series posted by you potentially months later to figure out what's current.
Is there a udf git tree?
On kernel.org which is down... I didn't bother to setup some other place
yet.
Quoted 2 lines
The stuff on sourceforge is not current and the
MAINTAINERSW: entry doesn't work.
Actually, the link is ancient and I don't use SF for UDF development so
I've added a patch to my tree which removes the link.
Quoted 20 lines
Maybe:
MAINTAINERS +1
-1@@ -6624,7 +6624,7 @@ F: drivers/net/ethernet/8390/ne-h8300.c UDF FILESYSTEM M: Jan Kara <[email protected]> -W: http://linux-udf.sourceforge.net +W: http://sourceforge.net/projects/linux-udf/develop S: Maintained F: Documentation/filesystems/udf.txt F: fs/udf/cheers, Joe
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
PATCH 5/6 udf: Neaten logging output, use vsprintf extension %pV
In reply to Joe PerchesRemove the warning and error prefixes that duplicate KERN_<level>. Use %pV and remove a static buffer to save some text space.
--- fs/udf/super.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-)
fs/udf/super.c +17 -11
@@ -75,8 +75,6 @@#define UDF_DEFAULT_BLOCKSIZE 2048 -static char error_buf[1024 ]; -/* These are the "meat" - everything else is stuffing */ static int udf_fill_super (struct super_block *,void *,int );static void udf_put_super (struct super_block *);
@@ -2075,29 +2073,37 @@ error_out:void _udf_err (struct super_block *sb,const char *function,const char *fmt, ...) { +struct va_format vaf;va_list args; -if (!(sb->s_flags &MS_RDONLY )) { -/* mark sb error */ +/* mark sb error */ +if (!(sb->s_flags &MS_RDONLY )) sb->s_dirt =1 ; - } +va_start (args, fmt); -vsnprintf (error_buf,sizeof (error_buf), fmt, args); + + vaf.fmt = fmt; + vaf.va = &args; + +pr_crit ("device %s: %s: %pV" , sb->s_id, function, &vaf); +va_end (args); -pr_crit ("error (device %s): %s: %s" , - sb->s_id, function, error_buf); }void _udf_warn (struct super_block *sb,const char *function,const char *fmt, ...) { +struct va_format vaf;va_list args;va_start (args, fmt); -vsnprintf (error_buf,sizeof (error_buf), fmt, args); + + vaf.fmt = fmt; + vaf.va = &args; + +pr_warn ("device %s: %s: %pV" , sb->s_id, function, &vaf); +va_end (args); -pr_warn ("warning (device %s): %s: %s" , - sb->s_id, function, error_buf); }static void udf_put_super (struct super_block *sb)
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 5/6] udf: Neaten logging output, use vsprintf extension %pV
2011/10/10 Joe Perches <[email protected]>:
Quoted 4 lines
Remove the warning and error prefixes that duplicate KERN_<level>. Use %pV and remove a static buffer to save some text space.
Signed-off-by Joe Perches <[email protected]>
Quoted 1 line
---
Re: [PATCH 5/6] udf: Neaten logging output, use vsprintf extension %pV
In reply to Joe PerchesQuoted 1 line · Joe Perches
Remove the warning and error prefixes that duplicate KERN_<level>.
Please keep that in the prefix. It's easier to read messages when they
are that way ...
Quoted 1 line
Use %pV and remove a static buffer to save some text space.
This is actually a bugfix because we could corrupt error message when
several error messages would be reported in parallel... Thanks for finding this.
Honza
Quoted 66 lines
Signed-off-by Joe Perches <[email protected]>--- fs/udf/super.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-)fs/udf/super.c +17
-11@@ -75,8 +75,6 @@#define UDF_DEFAULT_BLOCKSIZE 2048 -static char error_buf[1024 ]; -/* These are the "meat" - everything else is stuffing */ static int udf_fill_super (struct super_block *,void *,int );static void udf_put_super (struct super_block *);@@ -2075,29 +2073,37 @@ error_out:void _udf_err (struct super_block *sb,const char *function,const char *fmt, ...) { +struct va_format vaf;va_list args; -if (!(sb->s_flags &MS_RDONLY )) { -/* mark sb error */ +/* mark sb error */ +if (!(sb->s_flags &MS_RDONLY )) sb->s_dirt =1 ; - } +va_start (args, fmt); -vsnprintf (error_buf,sizeof (error_buf), fmt, args); + + vaf.fmt = fmt; + vaf.va = &args; + +pr_crit ("device %s: %s: %pV" , sb->s_id, function, &vaf); +va_end (args); -pr_crit ("error (device %s): %s: %s" , - sb->s_id, function, error_buf); }void _udf_warn (struct super_block *sb,const char *function,const char *fmt, ...) { +struct va_format vaf;va_list args;va_start (args, fmt); -vsnprintf (error_buf,sizeof (error_buf), fmt, args); + + vaf.fmt = fmt; + vaf.va = &args; + +pr_warn ("device %s: %s: %pV" , sb->s_id, function, &vaf); +va_end (args); -pr_warn ("warning (device %s): %s: %s" , - sb->s_id, function, error_buf); }static void udf_put_super (struct super_block *sb)
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Re: [PATCH 5/6] udf: Neaten logging output, use vsprintf extension %pV
Quoted 4 lines · Jan Kara
Quoted 1 line · Joe Perches
Remove the warning and error prefixes that duplicate KERN_<level>.
Please keep that in the prefix. It's easier to read messages when they
are that way ...
I think duplicating kernel standard functionality is not good and this merely increases logging buffer size.
It's your code, you're the maintainer, fix it up
as you choose.
PATCH 6/6 udf: Neaten udf_debug uses
In reply to Joe PerchesJust whitespace and argument alignment.
Introduce some checkpatch warnings that deserve to be ignored.
--- fs/udf/balloc.c | 14 +++++---- fs/udf/directory.c | 4 +- fs/udf/inode.c | 3 +- fs/udf/lowlevel.c | 2 +- fs/udf/partition.c | 8 ++-- fs/udf/super.c | 78 ++++++++++++++++++++++----------------------------- 6 files changed, 50 insertions(+), 59 deletions(-)
fs/udf/balloc.c +8 -6
@@ -59,8 +59,8 @@ static int __load_block_bitmap(struct super_block *sb,int nr_groups = bitmap->s_nr_groups;if (block_group >= nr_groups) { -udf_debug ("block_group (%d) > nr_groups (%d)\n" , block_group, - nr_groups); +udf_debug ("block_group (%d) > nr_groups (%d)\n" , + block_group, nr_groups); }if (bitmap->s_block_bitmap[block_group]) {
@@ -126,8 +126,9 @@ static void udf_bitmap_free_blocks(struct super_block *sb,if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) {udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, - count, partmap->s_partition_len); + bloc->logicalBlockNum,0 , + bloc->logicalBlockNum, count, + partmap->s_partition_len); goto error_return; }
@@ -155,7 +156,7 @@ static void udf_bitmap_free_blocks(struct super_block *sb,if (udf_set_bit (bit + i, bh->b_data)) {udf_debug ("bit %ld already set\n" , bit + i);udf_debug ("byte=%2x\n" , - ((char *)bh->b_data)[(bit + i) >>3 ]); + ((char *)bh->b_data)[(bit + i) >>3 ]); } }udf_add_free_space (sb, sbi->s_partition, count);
@@ -369,7 +370,8 @@ static void udf_table_free_blocks(struct super_block *sb,if (bloc->logicalBlockNum + count < count || (bloc->logicalBlockNum + count) > partmap->s_partition_len) {udf_debug ("%d < %d || %d + %d > %d\n" , - bloc->logicalBlockNum,0 , bloc->logicalBlockNum, count, + bloc->logicalBlockNum,0 , + bloc->logicalBlockNum, count, partmap->s_partition_len); goto error_return; }
fs/udf/directory.c +2 -2
@@ -162,8 +162,8 @@ struct fileIdentDesc *udf_get_fileident(void *buffer, int bufsize, int *offset)int padlen;if ((!buffer) || (!offset)) { -udf_debug ("invalidparms\n, buffer=%p, offset=%p\n" , buffer, - offset); +udf_debug ("invalidparms, buffer=%p, offset=%p\n" , + buffer, offset);return NULL ; }
fs/udf/inode.c +1 -2
@@ -1977,8 +1977,7 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos, *elen =le32_to_cpu (lad->extLength) &UDF_EXTENT_LENGTH_MASK ;break ;default : -udf_debug ("alloc_type = %d unsupported\n" , - iinfo->i_alloc_type); +udf_debug ("alloc_type = %d unsupported\n" , iinfo->i_alloc_type);return -1 ; }
fs/udf/lowlevel.c +1 -1
@@ -38,7 +38,7 @@ unsigned int udf_get_last_session(struct super_block *sb)if (i ==0 ) {udf_debug ("XA disk: %s, vol_desc_start=%d\n" , - (ms_info.xa_flag ?"yes" :"no" ), ms_info.addr.lba); + ms_info.xa_flag ?"yes" :"no" , ms_info.addr.lba);if (ms_info.xa_flag)/* necessary for a valid ms_info.addr */ vol_desc_start = ms_info.addr.lba; }else {
fs/udf/partition.c +4 -4
@@ -33,8 +33,8 @@ uint32_t udf_get_pblock(struct super_block *sb, uint32_t block,struct udf_sb_info *sbi =UDF_SB (sb);struct udf_part_map *map;if (partition >= sbi->s_partitions) { -udf_debug ("block=%d, partition=%d, offset=%d: " -"invalid partition\n" , block, partition, offset); +udf_debug ("block=%d, partition=%d, offset=%d: invalid partition\n" , + block, partition, offset);return 0xFFFFFFFF ; } map = &sbi->s_partmaps[partition];
@@ -60,8 +60,8 @@ uint32_t udf_get_pblock_virt15(struct super_block *sb, uint32_t block, vdata = &map->s_type_specific.s_virtual;if (block > vdata->s_num_entries) { -udf_debug ("Trying to access block beyond end of VAT " -"(%d max %d)\n" , block, vdata->s_num_entries); +udf_debug ("Trying to access block beyond end of VAT (%d max %d)\n" , + block, vdata->s_num_entries);return 0xFFFFFFFF ; }
fs/udf/super.c +34 -44
195 lines not on this page · raw message
Signature · 1 line
1.7.6.405.gc1be0
Re: [PATCH 6/6] udf: Neaten udf_debug uses
2011/10/10 Joe Perches <[email protected]>:
Quoted 4 lines
Just whitespace and argument alignment.
Introduce some checkpatch warnings that deserve to be ignored.Signed-off-by Joe Perches <[email protected]>
Quoted 1 line
---
Re: [PATCH 0/6] udf: Change some KERN_<level>s and logging neatening
In reply to Joe Perches2011/10/10 Joe Perches <[email protected]>:
Quoted 11 lines
I reviewed your patch and think it's too large for others to review because it combines multiple things into a single patch.
I think that adding struct
super_block* toudb_debugisn't necessary or particularly useful.Here are the changes to your original patch that I made:
Separate a single large patch into reviewable chunks. Don't convert every message type to udf_<foo>.
Hi Joe.
Thank you, you are given a nice clean up.
Quoted 27 lines
Joe Perches (6):
udf: Promote some debugging messages toudf_error
udf: Renameudf_errortoudf_err
udf: Renameudf_warntoudf_warn
udf: Convert printks to pr_<level>
udf: Neaten logging output, use vsprintf extension %pV
udf: Neatenudf_debugusesfs/udf/balloc.c | 14 ++--
fs/udf/directory.c | 8 +-
fs/udf/inode.c | 36 ++++-----
fs/udf/lowlevel.c | 2 +-
fs/udf/misc.c | 24 ++++---
fs/udf/namei.c | 5 +-
fs/udf/partition.c | 11 +--
fs/udf/super.c | 214 +++++++++++++++++++++++-----------------------------fs/udf/truncate.c | 21 ++--- fs/udf/udfdecl.h | 35 ++++++--- fs/udf/udftime.c | 3 +- fs/udf/unicode.c | 6 +- 12 files changed, 178 insertions(+), 201 deletions(-)--
1.7.6.405.gc1be0
Re: [PATCH 0/6] udf: Change some KERN_<level>s and logging neatening
In reply to Joe PerchesQuoted 12 lines · Joe Perches
I reviewed your patch and think it's too large for others to review because it combines multiple things into a single patch.
I think that adding struct
super_block* toudb_debugisn't necessary or particularly useful.Here are the changes to your original patch that I made:
Separate a single large patch into reviewable chunks. Don't convert every message type to udf_<foo>.
Quoted 7 lines
Joe Perches (6):
udf: Promote some debugging messages toudf_error
udf: Renameudf_errortoudf_err
udf: Renameudf_warntoudf_warn
udf: Convert printks to pr_<level>
udf: Neaten logging output, use vsprintf extension %pV
udf: Neatenudf_debuguses
So, I've merged all the patches (patches 1, 4, and 5 with some
modifications). Since kernel.org where I normally have my tree is down (well, better said I have not reestablished my tree there yet), I'll resend the whole series in a while so that people see what I carry.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR