mtd: ubi: Extend UBI layer debug/messaging capabilities
Subsystems (4)
PATCH v5 1/1 mtd: ubi: Extend UBI layer debug/messaging capabilities
If there is more then one UBI device mounted, there is no way to distinguish between messages from different UBI devices. Add device number to all ubi layer message types.
The R/O block driver messages were replaced by pr_* since ubi_device structure is not used by it.
---
Changes from V1:
- Compilation error fixed
- macros were updated to receive the ubi structure as parameter and not just ubi_number
- Places in code, where ubi-messaging macros were used, but ubi struct is not present yet (init phase), were updated to just use pr_err or similar
Changes from V2:
- multi line messages layout, as requested by Artem note that due to these changes checkpatch fails
Changes from V3:
- Compilation fix of block.c
- All ubi_* messages in block.c were replaced with pr_*
Changes from V4:
- Addressed comments on block.c
Note for reviewers:
The updated macros are all in ubi.h. All other file changes are just technical changes for compilation (usage of the defined macros)
drivers/mtd/ubi/attach.c | 126 +++++++++++++++++++------------------- drivers/mtd/ubi/block.c | 33 +++++----- drivers/mtd/ubi/build.c | 122 ++++++++++++++++++++----------------- drivers/mtd/ubi/cdev.c | 36 +++++------ drivers/mtd/ubi/debug.c | 8 +-- drivers/mtd/ubi/eba.c | 54 +++++++++-------- drivers/mtd/ubi/fastmap.c | 96 ++++++++++++++--------------- drivers/mtd/ubi/io.c | 150 ++++++++++++++++++++++++---------------------- drivers/mtd/ubi/kapi.c | 6 +- drivers/mtd/ubi/misc.c | 4 +- drivers/mtd/ubi/ubi.h | 13 ++-- drivers/mtd/ubi/vmt.c | 69 +++++++++++---------- drivers/mtd/ubi/vtbl.c | 49 ++++++++------- drivers/mtd/ubi/wl.c | 70 ++++++++++++---------- 14 files changed, 437 insertions(+), 399 deletions(-)
drivers/mtd/ubi/attach.c +66 -60
@@ -176,6 +176,7 @@ static int add_corrupted(struct ubi_attach_info *ai, int pnum, int ec)/** * validate_vid_hdr - check volume identifier header. +* @ubi: UBI device description object * @vid_hdr: the volume identifier header to check * @av: information about the volume this logical eraseblock belongs to * @pnum: physical eraseblock number the VID header came from
@@ -188,7 +189,8 @@ static int add_corrupted(struct ubi_attach_info *ai, int pnum, int ec)* information in the VID header is consistent to the information in other VID * headers of the same volume. */ -static int validate_vid_hdr (const struct ubi_vid_hdr *vid_hdr, +static int validate_vid_hdr (const struct ubi_device *ubi, +const struct ubi_vid_hdr *vid_hdr,const struct ubi_ainf_volume *av,int pnum) {int vol_type = vid_hdr->vol_type;
@@ -206,7 +208,7 @@ static int validate_vid_hdr(const struct ubi_vid_hdr *vid_hdr,*/ if (vol_id != av->vol_id) { -ubi_err ("inconsistent vol_id" ); +ubi_err (ubi,"inconsistent vol_id" ); goto bad; }
@@ -216,17 +218,17 @@ static int validate_vid_hdr(const struct ubi_vid_hdr *vid_hdr, av_vol_type =UBI_VID_DYNAMIC ;if (vol_type != av_vol_type) { -ubi_err ("inconsistent vol_type" ); +ubi_err (ubi,"inconsistent vol_type" ); goto bad; }if (used_ebs != av->used_ebs) { -ubi_err ("inconsistent used_ebs" ); +ubi_err (ubi,"inconsistent used_ebs" ); goto bad; }if (data_pad != av->data_pad) { -ubi_err ("inconsistent data_pad" ); +ubi_err (ubi,"inconsistent data_pad" ); goto bad; } }
@@ -234,7 +236,7 @@ static int validate_vid_hdr(const struct ubi_vid_hdr *vid_hdr,return 0 ; bad: -ubi_err ("inconsistent VID header at PEB %d" , pnum); +ubi_err (ubi,"inconsistent VID header at PEB %d" , pnum);ubi_dump_vid_hdr (vid_hdr);ubi_dump_av (av);return -EINVAL ;
@@ -336,7 +338,7 @@ int ubi_compare_lebs(struct ubi_device *ubi, const struct ubi_ainf_peb *aeb,* support these images anymore. Well, those images still work, * but only if no unclean reboots happened. */ -ubi_err ("unsupported on-flash UBI format" ); +ubi_err (ubi,"unsupported on-flash UBI format" );return -EINVAL ; }
@@ -377,7 +379,7 @@ int ubi_compare_lebs(struct ubi_device *ubi, const struct ubi_ainf_peb *aeb,if (err ==UBI_IO_BITFLIPS ) bitflips =1 ;else { -ubi_err ("VID of PEB %d header is bad, but it was OK earlier, err %d" , +ubi_err (ubi,"VID of PEB %d header is bad, but it was OK earlier, err %d" , pnum, err);if (err >0 ) err = -EIO ;
@@ -507,7 +509,7 @@ int ubi_add_to_av(struct ubi_device *ubi, struct ubi_attach_info *ai, int pnum,* logical eraseblocks because there was an unclean reboot. */ if (aeb->sqnum == sqnum && sqnum !=0 ) { -ubi_err ("two LEBs with same sequence number %llu" , +ubi_err (ubi,"two LEBs with same sequence number %llu" , sqnum);ubi_dump_aeb (aeb,0 );ubi_dump_vid_hdr (vid_hdr);
@@ -527,7 +529,7 @@ int ubi_add_to_av(struct ubi_device *ubi, struct ubi_attach_info *ai, int pnum,* This logical eraseblock is newer than the one * found earlier. */ - err =validate_vid_hdr (vid_hdr, av, pnum); + err =validate_vid_hdr (ubi, vid_hdr, av, pnum);if (err)return err;
@@ -565,7 +567,7 @@ int ubi_add_to_av(struct ubi_device *ubi, struct ubi_attach_info *ai, int pnum,* attaching information. */ - err =validate_vid_hdr (vid_hdr, av, pnum); + err =validate_vid_hdr (ubi, vid_hdr, av, pnum);if (err)return err;
@@ -668,7 +670,8 @@ static int early_erase_peb(struct ubi_device *ubi,* Erase counter overflow. Upgrade UBI and use 64-bit * erase counters internally. */ -ubi_err ("erase counter overflow at PEB %d, EC %d" , pnum, ec); +ubi_err (ubi,"erase counter overflow at PEB %d, EC %d" , + pnum, ec);return -EINVAL ; }
@@ -736,7 +739,7 @@ struct ubi_ainf_peb *ubi_early_get_peb(struct ubi_device *ubi,return aeb; } -ubi_err ("no free eraseblocks" ); +ubi_err (ubi,"no free eraseblocks" );return ERR_PTR (-ENOSPC ); }
@@ -785,9 +788,9 @@ static int check_corruption(struct ubi_device *ubi, struct ubi_vid_hdr *vid_hdr,if (ubi_check_pattern (ubi->peb_buf,0xFF , ubi->leb_size)) goto out_unlock; -ubi_err ("PEB %d contains corrupted VID header, and the data does not contain all 0xFF" , +ubi_err (ubi,"PEB %d contains corrupted VID header, and the data does not contain all 0xFF" , pnum); -ubi_err ("this may be a non-UBI PEB or a severe VID header corruption which requires manual inspection" ); +ubi_err (ubi,"this may be a non-UBI PEB or a severe VID header corruption which requires manual inspection" );ubi_dump_vid_hdr (vid_hdr);pr_err ("hexdump of PEB %d offset %d, length %d" , pnum, ubi->leb_start, ubi->leb_size);
@@ -859,7 +862,8 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai, bitflips =1 ;break ;default : -ubi_err ("'ubi_io_read_ec_hdr()' returned unknown code %d" , err); +ubi_err (ubi,"'ubi_io_read_ec_hdr()' returned unknown code %d" , + err);return -EINVAL ; }
@@ -868,7 +872,7 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,/* Make sure UBI version is OK */ if (ech->version !=UBI_VERSION ) { -ubi_err ("this UBI version is %d, image version is %d" , +ubi_err (ubi,"this UBI version is %d, image version is %d" ,UBI_VERSION , (int )ech->version);return -EINVAL ; }
@@ -882,7 +886,7 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,* flash. Upgrade UBI and use 64-bit erase counters * internally. */ -ubi_err ("erase counter overflow, max is %d" , +ubi_err (ubi,"erase counter overflow, max is %d" ,UBI_MAX_ERASECOUNTER );ubi_dump_ec_hdr (ech);return -EINVAL ;
@@ -903,7 +907,7 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,if (!ubi->image_seq) ubi->image_seq = image_seq;if (image_seq && ubi->image_seq != image_seq) { -ubi_err ("bad image sequence number %d in PEB %d, expected %d" , +ubi_err (ubi,"bad image sequence number %d in PEB %d, expected %d" , image_seq, pnum, ubi->image_seq);ubi_dump_ec_hdr (ech);return -EINVAL ;
@@ -981,7 +985,7 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,return err; goto adjust_mean_ec;default : -ubi_err ("'ubi_io_read_vid_hdr()' returned unknown code %d" , +ubi_err (ubi,"'ubi_io_read_vid_hdr()' returned unknown code %d" , err);return -EINVAL ; }
@@ -999,7 +1003,7 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,case UBI_COMPAT_DELETE :if (vol_id !=UBI_FM_SB_VOLUME_ID && vol_id !=UBI_FM_DATA_VOLUME_ID ) { -ubi_msg ("\"delete\" compatible internal volume %d:%d found, will remove it" , +ubi_msg (ubi,"\"delete\" compatible internal volume %d:%d found, will remove it" , vol_id, lnum); } err = add_to_list(ai, pnum, vol_id, lnum,
@@ -1009,13 +1013,13 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,return 0 ;case UBI_COMPAT_RO : -ubi_msg ("read-only compatible internal volume %d:%d found, switch to read-only mode" , +ubi_msg (ubi,"read-only compatible internal volume %d:%d found, switch to read-only mode" , vol_id, lnum); ubi->ro_mode =1 ;break ;case UBI_COMPAT_PRESERVE : -ubi_msg ("\"preserve\" compatible internal volume %d:%d found" , +ubi_msg (ubi,"\"preserve\" compatible internal volume %d:%d found" , vol_id, lnum); err =add_to_list (ai, pnum, vol_id, lnum, ec,0 , &ai->alien);
@@ -1024,14 +1028,14 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,return 0 ;case UBI_COMPAT_REJECT : -ubi_err ("incompatible internal volume %d:%d found" , +ubi_err (ubi,"incompatible internal volume %d:%d found" , vol_id, lnum);return -EINVAL ; } }if (ec_err) -ubi_warn ("valid VID header but corrupted EC header at PEB %d" , +ubi_warn (ubi,"valid VID header but corrupted EC header at PEB %d" , pnum); err =ubi_add_to_av (ubi, ai, pnum, ec, vidh, bitflips);if (err)
@@ -1075,7 +1079,7 @@ static int late_analysis(struct ubi_device *ubi, struct ubi_attach_info *ai)* with the flash HW or driver. */ if (ai->corr_peb_count) { -ubi_err ("%d PEBs are corrupted and preserved" , +ubi_err (ubi,"%d PEBs are corrupted and preserved" , ai->corr_peb_count);pr_err ("Corrupted PEBs are:" );list_for_each_entry (aeb, &ai->corr, u.list)
@@ -1087,7 +1091,7 @@ static int late_analysis(struct ubi_device *ubi, struct ubi_attach_info *ai)* otherwise, only print a warning. */ if (ai->corr_peb_count >= max_corr) { -ubi_err ("too many corrupted PEBs, refusing" ); +ubi_err (ubi,"too many corrupted PEBs, refusing" );return -EINVAL ; } }
@@ -1110,11 +1114,11 @@ static int late_analysis(struct ubi_device *ubi, struct ubi_attach_info *ai)*/ if (ai->maybe_bad_peb_count <=2 ) { ai->is_empty =1 ; -ubi_msg ("empty MTD device detected" ); +ubi_msg (ubi,"empty MTD device detected" );get_random_bytes (&ubi->image_seq,sizeof (ubi->image_seq)); }else { -ubi_err ("MTD device is not UBI-formatted and possibly contains non-UBI data - refusing it" ); +ubi_err (ubi,"MTD device is not UBI-formatted and possibly contains non-UBI data - refusing it" );return -EINVAL ; }
@@ -1248,7 +1252,7 @@ static int scan_all(struct ubi_device *ubi, struct ubi_attach_info *ai, goto out_vidh; } -ubi_msg ("scanning is finished" ); +ubi_msg (ubi,"scanning is finished" );/* Calculate mean erase counter */ if (ai->ec_count)
@@ -1515,37 +1519,37 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) vols_found +=1 ;if (ai->is_empty) { -ubi_err ("bad is_empty flag" ); +ubi_err (ubi,"bad is_empty flag" ); goto bad_av; }if (av->vol_id <0 || av->highest_lnum <0 || av->leb_count <0 || av->vol_type <0 || av->used_ebs <0 || av->data_pad <0 || av->last_data_size <0 ) { -ubi_err ("negative values" ); +ubi_err (ubi,"negative values" ); goto bad_av; }if (av->vol_id >=UBI_MAX_VOLUMES && av->vol_id <UBI_INTERNAL_VOL_START ) { -ubi_err ("bad vol_id" ); +ubi_err (ubi,"bad vol_id" ); goto bad_av; }if (av->vol_id > ai->highest_vol_id) { -ubi_err ("highest_vol_id is %d, but vol_id %d is there" , +ubi_err (ubi,"highest_vol_id is %d, but vol_id %d is there" , ai->highest_vol_id, av->vol_id); goto out; }if (av->vol_type !=UBI_DYNAMIC_VOLUME && av->vol_type !=UBI_STATIC_VOLUME ) { -ubi_err ("bad vol_type" ); +ubi_err (ubi,"bad vol_type" ); goto bad_av; }if (av->data_pad > ubi->leb_size /2 ) { -ubi_err ("bad data_pad" ); +ubi_err (ubi,"bad data_pad" ); goto bad_av; }
@@ -1557,48 +1561,48 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) leb_count +=1 ;if (aeb->pnum <0 || aeb->ec <0 ) { -ubi_err ("negative values" ); +ubi_err (ubi,"negative values" ); goto bad_aeb; }if (aeb->ec < ai->min_ec) { -ubi_err ("bad ai->min_ec (%d), %d found" , +ubi_err (ubi,"bad ai->min_ec (%d), %d found" , ai->min_ec, aeb->ec); goto bad_aeb; }if (aeb->ec > ai->max_ec) { -ubi_err ("bad ai->max_ec (%d), %d found" , +ubi_err (ubi,"bad ai->max_ec (%d), %d found" , ai->max_ec, aeb->ec); goto bad_aeb; }if (aeb->pnum >= ubi->peb_count) { -ubi_err ("too high PEB number %d, total PEBs %d" , +ubi_err (ubi,"too high PEB number %d, total PEBs %d" , aeb->pnum, ubi->peb_count); goto bad_aeb; }if (av->vol_type ==UBI_STATIC_VOLUME ) {if (aeb->lnum >= av->used_ebs) { -ubi_err ("bad lnum or used_ebs" ); +ubi_err (ubi,"bad lnum or used_ebs" ); goto bad_aeb; } }else {if (av->used_ebs !=0 ) { -ubi_err ("non-zero used_ebs" ); +ubi_err (ubi,"non-zero used_ebs" ); goto bad_aeb; } }if (aeb->lnum > av->highest_lnum) { -ubi_err ("incorrect highest_lnum or lnum" ); +ubi_err (ubi,"incorrect highest_lnum or lnum" ); goto bad_aeb; } }if (av->leb_count != leb_count) { -ubi_err ("bad leb_count, %d objects in the tree" , +ubi_err (ubi,"bad leb_count, %d objects in the tree" , leb_count); goto bad_av; }
@@ -1609,13 +1613,13 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) aeb = last_aeb;if (aeb->lnum != av->highest_lnum) { -ubi_err ("bad highest_lnum" ); +ubi_err (ubi,"bad highest_lnum" ); goto bad_aeb; } }if (vols_found != ai->vols_found) { -ubi_err ("bad ai->vols_found %d, should be %d" , +ubi_err (ubi,"bad ai->vols_found %d, should be %d" , ai->vols_found, vols_found); goto out; }
@@ -1632,7 +1636,8 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) err =ubi_io_read_vid_hdr (ubi, aeb->pnum, vidh,1 );if (err && err !=UBI_IO_BITFLIPS ) { -ubi_err ("VID header is not OK (%d)" , err); +ubi_err (ubi,"VID header is not OK (%d)" , + err);if (err >0 ) err = -EIO ;return err;
@@ -1641,37 +1646,37 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) vol_type = vidh->vol_type ==UBI_VID_DYNAMIC ?UBI_DYNAMIC_VOLUME :UBI_STATIC_VOLUME ;if (av->vol_type != vol_type) { -ubi_err ("bad vol_type" ); +ubi_err (ubi,"bad vol_type" ); goto bad_vid_hdr; }if (aeb->sqnum !=be64_to_cpu (vidh->sqnum)) { -ubi_err ("bad sqnum %llu" , aeb->sqnum); +ubi_err (ubi,"bad sqnum %llu" , aeb->sqnum); goto bad_vid_hdr; }if (av->vol_id !=be32_to_cpu (vidh->vol_id)) { -ubi_err ("bad vol_id %d" , av->vol_id); +ubi_err (ubi,"bad vol_id %d" , av->vol_id); goto bad_vid_hdr; }if (av->compat != vidh->compat) { -ubi_err ("bad compat %d" , vidh->compat); +ubi_err (ubi,"bad compat %d" , vidh->compat); goto bad_vid_hdr; }if (aeb->lnum !=be32_to_cpu (vidh->lnum)) { -ubi_err ("bad lnum %d" , aeb->lnum); +ubi_err (ubi,"bad lnum %d" , aeb->lnum); goto bad_vid_hdr; }if (av->used_ebs !=be32_to_cpu (vidh->used_ebs)) { -ubi_err ("bad used_ebs %d" , av->used_ebs); +ubi_err (ubi,"bad used_ebs %d" , av->used_ebs); goto bad_vid_hdr; }if (av->data_pad !=be32_to_cpu (vidh->data_pad)) { -ubi_err ("bad data_pad %d" , av->data_pad); +ubi_err (ubi,"bad data_pad %d" , av->data_pad); goto bad_vid_hdr; } }
@@ -1680,12 +1685,13 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)continue ;if (av->highest_lnum !=be32_to_cpu (vidh->lnum)) { -ubi_err ("bad highest_lnum %d" , av->highest_lnum); +ubi_err (ubi,"bad highest_lnum %d" , av->highest_lnum); goto bad_vid_hdr; }if (av->last_data_size !=be32_to_cpu (vidh->data_size)) { -ubi_err ("bad last_data_size %d" , av->last_data_size); +ubi_err (ubi,"bad last_data_size %d" , + av->last_data_size); goto bad_vid_hdr; } }
@@ -1726,7 +1732,7 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) err =0 ;for (pnum =0 ; pnum < ubi->peb_count; pnum++)if (!buf[pnum]) { -ubi_err ("PEB %d is not referred" , pnum); +ubi_err (ubi,"PEB %d is not referred" , pnum); err =1 ; }
@@ -1736,18 +1742,18 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)return 0 ; bad_aeb: -ubi_err ("bad attaching information about LEB %d" , aeb->lnum); +ubi_err (ubi,"bad attaching information about LEB %d" , aeb->lnum);ubi_dump_aeb (aeb,0 );ubi_dump_av (av); goto out; bad_av: -ubi_err ("bad attaching information about volume %d" , av->vol_id); +ubi_err (ubi,"bad attaching information about volume %d" , av->vol_id);ubi_dump_av (av); goto out; bad_vid_hdr: -ubi_err ("bad attaching information about volume %d" , av->vol_id); +ubi_err (ubi,"bad attaching information about volume %d" , av->vol_id);ubi_dump_av (av);ubi_dump_vid_hdr (vidh);
drivers/mtd/ubi/block.c +16 -17
@@ -111,12 +111,12 @@ static int __init ubiblock_set_param(const char *val, len =strnlen (val,UBIBLOCK_PARAM_LEN );if (len ==0 ) { -ubi_warn ("block: empty 'block=' parameter - ignored\n" ); +pr_warn ("UBI: block: empty 'block=' parameter - ignored\n" );return 0 ; }if (len ==UBIBLOCK_PARAM_LEN ) { -ubi_err ("block: parameter \"%s\" is too long, max. is %d\n" , +pr_err ("UBI: block: parameter \"%s\" is too long, max. is %d\n" , val,UBIBLOCK_PARAM_LEN );return -EINVAL ; }
@@ -188,9 +188,8 @@ static int ubiblock_read_to_buf(struct ubiblock *dev, char *buffer, ret =ubi_read (dev->desc, leb, buffer, offset, len);if (ret) { -ubi_err ("%s: error %d while reading from LEB %d (offset %d, " -"length %d)" , dev->gd->disk_name, ret, leb, offset, - len); +dev_err (disk_to_dev (dev->gd),"%d while reading from LEB %d (offset %d, length %d)" , + ret, leb, offset, len);return ret; }return 0 ;
@@ -328,8 +327,8 @@ static int ubiblock_open(struct block_device *bdev, fmode_t mode) dev->desc =ubi_open_volume (dev->ubi_num, dev->vol_id,UBI_READONLY );if (IS_ERR (dev->desc)) { -ubi_err ("%s failed to open ubi volume %d_%d" , - dev->gd->disk_name, dev->ubi_num, dev->vol_id); +dev_err (disk_to_dev (dev->gd),"failed to open ubi volume %d_%d" , + dev->ubi_num, dev->vol_id); ret =PTR_ERR (dev->desc); dev->desc =NULL ; goto out_unlock;
@@ -405,7 +404,7 @@ int ubiblock_create(struct ubi_volume_info *vi)/* Initialize the gendisk of this ubiblock device */ gd =alloc_disk (1 );if (!gd) { -ubi_err ("block: alloc_disk failed" ); +pr_err ("UBI: block: alloc_disk failed" ); ret = -ENODEV ; goto out_free_dev; }
@@ -421,7 +420,7 @@ int ubiblock_create(struct ubi_volume_info *vi)spin_lock_init (&dev->queue_lock); dev->rq =blk_init_queue (ubiblock_request, &dev->queue_lock);if (!dev->rq) { -ubi_err ("block: blk_init_queue failed" ); +dev_err (disk_to_dev (gd),"blk_init_queue failed" ); ret = -ENODEV ; goto out_put_disk; }
@@ -446,8 +445,8 @@ int ubiblock_create(struct ubi_volume_info *vi)/* Must be the last step: anyone can call file ops from now on */ add_disk (dev->gd); -ubi_msg ("%s created from ubi%d:%d(%s)" , - dev->gd->disk_name, dev->ubi_num, dev->vol_id, vi->name); +dev_info (disk_to_dev (dev->gd),"created from ubi%d:%d(%s)" , + dev->ubi_num, dev->vol_id, vi->name);return 0 ; out_free_queue:
@@ -464,7 +463,7 @@ static void ubiblock_cleanup(struct ubiblock *dev) {del_gendisk (dev->gd);blk_cleanup_queue (dev->rq); -ubi_msg ("%s released" , dev->gd->disk_name); +dev_info (disk_to_dev (dev->gd),"released" );put_disk (dev->gd); }
@@ -518,8 +517,8 @@ static int ubiblock_resize(struct ubi_volume_info *vi) }if ((sector_t )disk_capacity != disk_capacity) {mutex_unlock (&devices_mutex); -ubi_warn ("%s: the volume is too big (%d LEBs), cannot resize" , - dev->gd->disk_name, vi->size); +dev_warn (disk_to_dev (dev->gd),"the volume is too big (%d LEBs), cannot resize" , + vi->size);return -EFBIG ; }
@@ -527,7 +526,7 @@ static int ubiblock_resize(struct ubi_volume_info *vi)if (get_capacity (dev->gd) != disk_capacity) {set_capacity (dev->gd, disk_capacity); -ubi_msg ("%s resized to %lld bytes" , dev->gd->disk_name, +dev_info (disk_to_dev (dev->gd),"resized to %lld bytes" , vi->used_bytes); }mutex_unlock (&dev->dev_mutex);
@@ -596,7 +595,7 @@ static int __init ubiblock_create_from_param(void) desc =open_volume_desc (p->name, p->ubi_num, p->vol_id);if (IS_ERR (desc)) { -ubi_err ("block: can't open volume, err=%ld\n" , +pr_err ("UBI: block: can't open volume, err=%ld\n" ,PTR_ERR (desc)); ret =PTR_ERR (desc);break ;
@@ -607,7 +606,7 @@ static int __init ubiblock_create_from_param(void) ret =ubiblock_create (&vi);if (ret) { -ubi_err ("block: can't add '%s' volume, err=%d\n" , +pr_err ("UBI: block: can't add '%s' volume, err=%d\n" , vi.name, ret);break ; }
drivers/mtd/ubi/build.c +66 -56
391 lines not on this page · raw message
drivers/mtd/ubi/cdev.c +19 -17
@@ -48,13 +48,14 @@/** * get_exclusive - get exclusive access to an UBI volume. +* @ubi: UBI device description object * @desc: volume descriptor * * This function changes UBI volume open mode to "exclusive". Returns previous * mode value (positive integer) in case of success and a negative error code * in case of failure. */ -static int get_exclusive (struct ubi_volume_desc *desc) +static int get_exclusive (struct ubi_device *ubi,struct ubi_volume_desc *desc) {int users, err;struct ubi_volume *vol = desc->vol;
@@ -63,7 +64,7 @@ static int get_exclusive(struct ubi_volume_desc *desc) users = vol->readers + vol->writers + vol->exclusive;ubi_assert (users >0 );if (users >1 ) { -ubi_err ("%d users for volume %d" , users, vol->vol_id); +ubi_err (ubi,"%d users for volume %d" , users, vol->vol_id); err = -EBUSY ; }else { vol->readers = vol->writers =0 ;
@@ -134,7 +135,7 @@ static int vol_cdev_release(struct inode *inode, struct file *file) vol->ubi->ubi_num, vol->vol_id, desc->mode);if (vol->updating) { -ubi_warn ("update of volume %d not finished, volume is damaged" , +ubi_warn (vol->ubi,"update of volume %d not finished, volume is damaged" , vol->vol_id);ubi_assert (!vol->changing_leb); vol->updating =0 ;
@@ -158,7 +159,7 @@ static loff_t vol_cdev_llseek(struct file *file, loff_t offset, int origin)if (vol->updating) {/* Update is in progress, seeking is prohibited */ -ubi_err ("updating" ); +ubi_err (vol->ubi,"updating" );return -EBUSY ; }
@@ -193,11 +194,11 @@ static ssize_t vol_cdev_read(struct file *file, __user char *buf, size_t count, count, *offp, vol->vol_id);if (vol->updating) { -ubi_err ("updating" ); +ubi_err (vol->ubi,"updating" );return -EBUSY ; }if (vol->upd_marker) { -ubi_err ("damaged volume, update marker is set" ); +ubi_err (vol->ubi,"damaged volume, update marker is set" );return -EBADF ; }if (*offp == vol->used_bytes || count ==0 )
@@ -277,7 +278,7 @@ static ssize_t vol_cdev_direct_write(struct file *file, const char __user *buf, lnum =div_u64_rem (*offp, vol->usable_leb_size, &off);if (off & (ubi->min_io_size -1 )) { -ubi_err ("unaligned position" ); +ubi_err (ubi,"unaligned position" );return -EINVAL ; }
@@ -286,7 +287,7 @@ static ssize_t vol_cdev_direct_write(struct file *file, const char __user *buf,/* We can write only in fractions of the minimum I/O unit */ if (count & (ubi->min_io_size -1 )) { -ubi_err ("unaligned write length" ); +ubi_err (ubi,"unaligned write length" );return -EINVAL ; }
@@ -348,7 +349,7 @@ static ssize_t vol_cdev_write(struct file *file, const char __user *buf, err =ubi_more_leb_change_data (ubi, vol, buf, count);if (err <0 ) { -ubi_err ("cannot accept more %zd bytes of data, error %d" , +ubi_err (ubi,"cannot accept more %zd bytes of data, error %d" , count, err);return err; }
@@ -370,7 +371,7 @@ static ssize_t vol_cdev_write(struct file *file, const char __user *buf,return err;if (err) { -ubi_warn ("volume %d on UBI device %d is corrupted" , +ubi_warn (ubi,"volume %d on UBI device %d is corrupted" , vol->vol_id, ubi->ubi_num); vol->corrupted =1 ; }
@@ -420,7 +421,7 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd,break ; } - err =get_exclusive (desc); + err =get_exclusive (ubi, desc);if (err <0 )break ;
@@ -456,7 +457,7 @@ static long vol_cdev_ioctl(struct file *file, unsigned int cmd, req.bytes < 0 || req.lnum >= vol->usable_leb_size)break ; - err =get_exclusive (desc); + err =get_exclusive (ubi, desc);if (err <0 )break ;
@@ -642,7 +643,7 @@ static int verify_mkvol_req(const struct ubi_device *ubi,return 0 ; bad: -ubi_err ("bad volume creation request" ); +ubi_err (ubi,"bad volume creation request" );ubi_dump_mkvol_req (req);return err; }
@@ -708,12 +709,12 @@ static int rename_volumes(struct ubi_device *ubi,for (i =0 ; i < req->count -1 ; i++) {for (n = i +1 ; n < req->count; n++) {if (req->ents[i].vol_id == req->ents[n].vol_id) { -ubi_err ("duplicated volume id %d" , +ubi_err (ubi,"duplicated volume id %d" , req->ents[i].vol_id);return -EINVAL ; }if (!strcmp (req->ents[i].name, req->ents[n].name)) { -ubi_err ("duplicated volume name \"%s\"" , +ubi_err (ubi,"duplicated volume name \"%s\"" , req->ents[i].name);return -EINVAL ; }
@@ -736,7 +737,8 @@ static int rename_volumes(struct ubi_device *ubi, re->desc =ubi_open_volume (ubi->ubi_num, vol_id,UBI_READWRITE );if (IS_ERR (re->desc)) { err =PTR_ERR (re->desc); -ubi_err ("cannot open volume %d, error %d" , vol_id, err); +ubi_err (ubi,"cannot open volume %d, error %d" , + vol_id, err);kfree (re); goto out_free; }
@@ -795,7 +797,7 @@ static int rename_volumes(struct ubi_device *ubi,continue ;/* The volume exists but busy, or an error occurred */ -ubi_err ("cannot open volume \"%s\", error %d" , +ubi_err (ubi,"cannot open volume \"%s\", error %d" , re->new_name, err); goto out_free; }
drivers/mtd/ubi/debug.c +4 -4
@@ -43,12 +43,12 @@ void ubi_dump_flash(struct ubi_device *ubi, int pnum, int offset, int len)return ; err =mtd_read (ubi->mtd, addr, len, &read, buf);if (err && err != -EUCLEAN ) { -ubi_err ("error %d while reading %d bytes from PEB %d:%d, read %zd bytes" , +ubi_err (ubi,"err %d while reading %d bytes from PEB %d:%d, read %zd bytes" , err, len, pnum, offset, read); goto out; } -ubi_msg ("dumping %d bytes of data from PEB %d, offset %d" , +ubi_msg (ubi,"dumping %d bytes of data from PEB %d, offset %d" , len, pnum, offset);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); out:
@@ -238,7 +238,7 @@ int ubi_debugfs_init(void)if (IS_ERR_OR_NULL (dfs_rootdir)) {int err = dfs_rootdir ? -ENODEV :PTR_ERR (dfs_rootdir); -ubi_err ("cannot create \"ubi\" debugfs directory, error %d\n" , +pr_err ("UBI error: cannot create \"ubi\" debugfs directory, error %d\n" , err);return err; }
@@ -433,7 +433,7 @@ out_remove:debugfs_remove_recursive (d->dfs_dir); out: err = dent ?PTR_ERR (dent) : -ENODEV ; -ubi_err ("cannot create \"%s\" debugfs file or directory, error %d\n" , +ubi_err (ubi,"cannot create \"%s\" debugfs file or directory, error %d\n" , fname, err);return err; }
drivers/mtd/ubi/eba.c +28 -26
201 lines not on this page · raw message
drivers/mtd/ubi/fastmap.c +48 -48
326 lines not on this page · raw message
drivers/mtd/ubi/io.c +77 -73
519 lines not on this page · raw message
drivers/mtd/ubi/kapi.c +3 -3
@@ -204,7 +204,7 @@ struct ubi_volume_desc *ubi_open_volume(int ubi_num, int vol_id, int mode)return ERR_PTR (err); }if (err ==1 ) { -ubi_warn ("volume %d on UBI device %d is corrupted" , +ubi_warn (ubi,"volume %d on UBI device %d is corrupted" , vol_id, ubi->ubi_num); vol->corrupted =1 ; }
@@ -221,7 +221,7 @@ out_free:kfree (desc); out_put_ubi:ubi_put_device (ubi); -ubi_err ("cannot open device %d, volume %d, error %d" , +ubi_err (ubi,"cannot open device %d, volume %d, error %d" , ubi_num, vol_id, err);return ERR_PTR (err); }
@@ -411,7 +411,7 @@ int ubi_leb_read(struct ubi_volume_desc *desc, int lnum, char *buf, int offset, err =ubi_eba_read_leb (ubi, vol, lnum, buf, offset, len, check);if (err &&mtd_is_eccerr (err) && vol->vol_type ==UBI_STATIC_VOLUME ) { -ubi_warn ("mark volume %d as corrupted" , vol_id); +ubi_warn (ubi,"mark volume %d as corrupted" , vol_id); vol->corrupted =1 ; }
drivers/mtd/ubi/misc.c +2 -2
@@ -111,7 +111,7 @@ void ubi_update_reserved(struct ubi_device *ubi) ubi->avail_pebs -= need; ubi->rsvd_pebs += need; ubi->beb_rsvd_pebs += need; -ubi_msg ("reserved more %d PEBs for bad PEB handling" , need); +ubi_msg (ubi,"reserved more %d PEBs for bad PEB handling" , need); }/**
@@ -128,7 +128,7 @@ void ubi_calculate_reserved(struct ubi_device *ubi) ubi->beb_rsvd_level = ubi->bad_peb_limit - ubi->bad_peb_count;if (ubi->beb_rsvd_level <0 ) { ubi->beb_rsvd_level =0 ; -ubi_warn ("number of bad PEBs (%d) is above the expected limit (%d), not reserving any PEBs for bad PEB handling, will use available PEBs (if any)" , +ubi_warn (ubi,"number of bad PEBs (%d) is above the expected limit (%d), not reserving any PEBs for bad PEB handling, will use available PEBs (if any)" , ubi->bad_peb_count, ubi->bad_peb_limit); } }
drivers/mtd/ubi/ubi.h +7 -6
@@ -50,13 +50,14 @@#define UBI_NAME_STR "ubi"/* Normal UBI messages */ -#define ubi_msg (fmt, ...) pr_notice("UBI: " fmt "\n", ##__VA_ARGS__) +#define ubi_msg (ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \ + ubi->ubi_num, __func__, ##__VA_ARGS__)/* UBI warning messages */ -#define ubi_warn (fmt, ...) pr_warn("UBI warning: %s: " fmt "\n", \ - __func__, ##__VA_ARGS__) +#define ubi_warn (ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \ + ubi->ubi_num, __func__, ##__VA_ARGS__)/* UBI error messages */ -#define ubi_err (fmt, ...) pr_err("UBI error: %s: " fmt "\n", \ - __func__, ##__VA_ARGS__) +#define ubi_err (ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \ + ubi->ubi_num, __func__, ##__VA_ARGS__)/* Background thread name pattern */ #define UBI_BGT_NAME_PATTERN "ubi_bgt%dd"
@@ -987,7 +988,7 @@ static inline void ubi_ro_mode(struct ubi_device *ubi) {if (!ubi->ro_mode) { ubi->ro_mode =1 ; -ubi_warn ("switch to read-only mode" ); +ubi_warn (ubi,"switch to read-only mode" );dump_stack (); } }
drivers/mtd/ubi/vmt.c +36 -33
229 lines not on this page · raw message
drivers/mtd/ubi/vtbl.c +26 -23
175 lines not on this page · raw message
drivers/mtd/ubi/wl.c +39 -31
234 lines not on this page · raw message
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Artem Bityutskiy
AppliedQuoted 8 lines · Tanya Brokhman
If there is more then one UBI device mounted, there is no way to distinguish between messages from different UBI devices. Add device number to all ubi layer message types.
The R/O block driver messages were replaced by pr_* since
ubi_devicestructure is not used by it.Signed-off-by Tanya Brokhman <[email protected]>
Pushed to linux-ubifs.git, thanks.
You did not get indentations right, though, so I amended your patch. Also, some lines were split unnecessarily. My amendments are in the diff below. Please, take this into account for the similar UBIFS patch which you was going to send. Thank you!
drivers/mtd/ubi/attach.c +3 -3
@@ -671,7 +671,7 @@ static int early_erase_peb(struct ubi_device *ubi,* erase counters internally. */ ubi_err (ubi,"erase counter overflow at PEB %d, EC %d" , - pnum, ec); + pnum, ec);return -EINVAL ; }
@@ -1637,7 +1637,7 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai) err =ubi_io_read_vid_hdr (ubi, aeb->pnum, vidh,1 );if (err && err !=UBI_IO_BITFLIPS ) {ubi_err (ubi,"VID header is not OK (%d)" , - err); + err);if (err >0 ) err = -EIO ;return err;
@@ -1691,7 +1691,7 @@ static int self_check_ai(struct ubi_device *ubi, struct ubi_attach_info *ai)if (av->last_data_size !=be32_to_cpu (vidh->data_size)) {ubi_err (ubi,"bad last_data_size %d" , - av->last_data_size); + av->last_data_size); goto bad_vid_hdr; } }
drivers/mtd/ubi/block.c +6 -6
@@ -117,7 +117,7 @@ static int __init ubiblock_set_param(const char *val,if (len ==UBIBLOCK_PARAM_LEN ) {pr_err ("UBI: block: parameter \"%s\" is too long, max. is %d\n" , - val,UBIBLOCK_PARAM_LEN ); + val,UBIBLOCK_PARAM_LEN );return -EINVAL ; }
@@ -446,7 +446,7 @@ int ubiblock_create(struct ubi_volume_info *vi)/* Must be the last step: anyone can call file ops from now on */ add_disk (dev->gd);dev_info (disk_to_dev (dev->gd),"created from ubi%d:%d(%s)" , - dev->ubi_num, dev->vol_id, vi->name); + dev->ubi_num, dev->vol_id, vi->name);return 0 ; out_free_queue:
@@ -518,7 +518,7 @@ static int ubiblock_resize(struct ubi_volume_info *vi)if ((sector_t )disk_capacity != disk_capacity) {mutex_unlock (&devices_mutex);dev_warn (disk_to_dev (dev->gd),"the volume is too big (%d LEBs), cannot resize" , - vi->size); + vi->size);return -EFBIG ; }
@@ -527,7 +527,7 @@ static int ubiblock_resize(struct ubi_volume_info *vi)if (get_capacity (dev->gd) != disk_capacity) {set_capacity (dev->gd, disk_capacity);dev_info (disk_to_dev (dev->gd),"resized to %lld bytes" , - vi->used_bytes); + vi->used_bytes); }mutex_unlock (&dev->dev_mutex);mutex_unlock (&devices_mutex);
@@ -596,7 +596,7 @@ static int __init ubiblock_create_from_param(void) desc =open_volume_desc (p->name, p->ubi_num, p->vol_id);if (IS_ERR (desc)) {pr_err ("UBI: block: can't open volume, err=%ld\n" , -PTR_ERR (desc)); +PTR_ERR (desc)); ret =PTR_ERR (desc);break ; }
@@ -607,7 +607,7 @@ static int __init ubiblock_create_from_param(void) ret =ubiblock_create (&vi);if (ret) {pr_err ("UBI: block: can't add '%s' volume, err=%d\n" , - vi.name, ret); + vi.name, ret);break ; } }
drivers/mtd/ubi/build.c +9 -11
@@ -960,8 +960,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ubi->fm_disabled = 1; } -ubi_msg (ubi,"default fastmap pool size: %d" , - ubi->fm_pool.max_size); +ubi_msg (ubi,"default fastmap pool size: %d" , ubi->fm_pool.max_size);ubi_msg (ubi,"default fastmap WL pool size: %d" , ubi->fm_wl_pool.max_size);#else
@@ -1224,7 +1223,7 @@ static int __init ubi_init(void)if (mtd_devs >UBI_MAX_DEVICES ) {pr_err ("UBI error: too many MTD devices, maximum is %d" , -UBI_MAX_DEVICES ); +UBI_MAX_DEVICES );return -EINVAL ; }
@@ -1272,7 +1271,7 @@ static int __init ubi_init(void)if (IS_ERR (mtd)) { err =PTR_ERR (mtd);pr_err ("UBI error: cannot open mtd %s, error %d" , - p->name, err); + p->name, err);/* See comment below re-ubi_is_module(). */ if (ubi_is_module ()) goto out_detach;
@@ -1285,7 +1284,7 @@ static int __init ubi_init(void)mutex_unlock (&ubi_devices_mutex);if (err <0 ) {pr_err ("UBI error: cannot attach mtd%d" , - mtd->index); + mtd->index);put_mtd_device (mtd);/*
@@ -1417,14 +1416,14 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)if (mtd_devs ==UBI_MAX_DEVICES ) {pr_err ("UBI error: too many parameters, max. is %d\n" , -UBI_MAX_DEVICES ); +UBI_MAX_DEVICES );return -EINVAL ; } len =strnlen (val,MTD_PARAM_LEN_MAX );if (len ==MTD_PARAM_LEN_MAX ) {pr_err ("UBI error: parameter \"%s\" is too long, max. is %d\n" , - val,MTD_PARAM_LEN_MAX ); + val,MTD_PARAM_LEN_MAX );return -EINVAL ; }
@@ -1443,8 +1442,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp) tokens[i] =strsep (&pbuf,"," );if (pbuf) { -pr_err ("UBI error: too many arguments at \"%s\"\n" , - val); +pr_err ("UBI error: too many arguments at \"%s\"\n" , val);return -EINVAL ; }
@@ -1465,7 +1463,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)if (err) {pr_err ("UBI error: bad value for max_beb_per1024 parameter: %s" , - token); + token);return -EINVAL ; } }
@@ -1476,7 +1474,7 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)if (err) {pr_err ("UBI error: bad value for ubi_num parameter: %s" , - token); + token);return -EINVAL ; } }else
drivers/mtd/ubi/cdev.c +1 -1
@@ -738,7 +738,7 @@ static int rename_volumes(struct ubi_device *ubi,if (IS_ERR (re->desc)) { err =PTR_ERR (re->desc);ubi_err (ubi,"cannot open volume %d, error %d" , - vol_id, err); + vol_id, err);kfree (re); goto out_free; }
drivers/mtd/ubi/debug.c +1 -1
@@ -239,7 +239,7 @@ int ubi_debugfs_init(void)int err = dfs_rootdir ? -ENODEV :PTR_ERR (dfs_rootdir);pr_err ("UBI error: cannot create \"ubi\" debugfs directory, error %d\n" , - err); + err);return err; }
drivers/mtd/ubi/eba.c +1 -2
@@ -614,8 +614,7 @@ int ubi_eba_write_leb(struct ubi_device *ubi, struct ubi_volume *vol, int lnum, err =ubi_io_write_data (ubi, buf, pnum, offset, len);if (err) { -ubi_warn (ubi,"failed to write data to PEB %d" , - pnum); +ubi_warn (ubi,"failed to write data to PEB %d" , pnum);if (err == -EIO && ubi->bad_allowed) err =recover_peb (ubi, pnum, vol_id, lnum, buf, offset, len);
drivers/mtd/ubi/io.c +1 -1
@@ -376,7 +376,7 @@ retry:if (ei.state ==MTD_ERASE_FAILED ) {if (retries++ <UBI_IO_RETRIES ) {ubi_warn (ubi,"error while erasing PEB %d, retry" , - pnum); + pnum);yield (); goto retry; }
drivers/mtd/ubi/ubi.h +5 -5
@@ -51,13 +51,13 @@/* Normal UBI messages */ #define ubi_msg (ubi, fmt, ...) pr_notice("UBI-%d: %s:" fmt "\n", \ - ubi->ubi_num, __func__, ##__VA_ARGS__) + ubi->ubi_num, __func__, ##__VA_ARGS__)/* UBI warning messages */ -#define ubi_warn (ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \ - ubi->ubi_num, __func__, ##__VA_ARGS__) +#define ubi_warn (ubi, fmt, ...) pr_warn("UBI-%d warning: %s: " fmt "\n", \ + ubi->ubi_num, __func__, ##__VA_ARGS__)/* UBI error messages */ -#define ubi_err (ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \ - ubi->ubi_num, __func__, ##__VA_ARGS__) +#define ubi_err (ubi, fmt, ...) pr_err("UBI-%d error: %s: " fmt "\n", \ + ubi->ubi_num, __func__, ##__VA_ARGS__)/* Background thread name pattern */ #define UBI_BGT_NAME_PATTERN "ubi_bgt%dd"
drivers/mtd/ubi/vtbl.c +1 -2
@@ -755,8 +755,7 @@ static int check_attaching_info(const struct ubi_device *ubi,* reboot while the volume was being removed. Discard * these eraseblocks. */ -ubi_msg (ubi,"finish volume %d removal" , - av->vol_id); +ubi_msg (ubi,"finish volume %d removal" , av->vol_id);ubi_remove_av (ai, av); }else if (av) { err =check_av (ubi, vol, av);
--
Artem.
Quoted 8 lines · Artem Bityutskiy
Quoted 7 lines · Tanya Brokhman
If there is more then one UBI device mounted, there is no way to distinguish between messages from different UBI devices. Add device number to all ubi layer message types.
The R/O block driver messages were replaced by pr_* since
ubi_devicestructure is not used by it.
Quoted 8 lines
Quoted 1 line
Signed-off-by Tanya Brokhman <[email protected]>Pushed to linux-ubifs.git, thanks.
You did not get indentations right, though, so I amended your patch. Also, some lines were split unnecessarily. My amendments are in the diff below. Please, take this into account for the similar UBIFS patch which you was going to send. Thank you!
Thanks Artem and sorry for the extra work for you. I was trying to find the balance between pleasing checkpatch and your request to have the message in the same line :) Will be extra careful about the indentation next time (guess my editor needs re-configuration). Sorry about that.
Thanks,
Tanya Brokhman
Signature · 2 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
It seems useful~!
I'd like to do some test with it~
Hi Tanya,
When I was trying to push this patch to my product, I reviewed this patch and found some small problems. I wish it's not too late to report these.
The patch I get from linux-ubifs.git is amended a bit by Artem. I'd like to quote your V5 patch for simplification. Some line numbers may mismatching.
Quoted 13 lines
drivers/mtd/ubi/build.c +1
-2@@ -1408,20 +1416,20 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)return -EINVAL ;if (mtd_devs ==UBI_MAX_DEVICES ) { -ubi_err ("too many parameters, max. is %d\n" , +pr_err ("UBI error: too many parameters, max. is %d\n" ,UBI_MAX_DEVICES );return -EINVAL ; } len =strnlen (val,MTD_PARAM_LEN_MAX );if (len ==MTD_PARAM_LEN_MAX ) { - ubi_err("parameter \"%s\" is too long, max. is %d\n",
drivers/mtd/ubi/build.c static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp) · line 1434
Quoted 10 lines
drivers/mtd/ubi/build.c +2
-1+pr_err ("UBI error: parameter \"%s\" is too long, max. is %d\n" , val,MTD_PARAM_LEN_MAX );return -EINVAL ; }if (len ==0 ) { -pr_warn ("UBI warning: empty 'mtd=' parameter - ignored\n" ); +pr_err ("UBI warning: empty 'mtd=' parameter - ignored\n" );return 0 ; }
Why the last 'pr_warn()' need to be changed into 'pr_err()'? I looked up your V1 and V2 patches, I think it's not your purpose.
Quoted 27 lines
drivers/mtd/ubi/attach.c +3
-0@@ -176,6 +176,7 @@ static int add_corrupted(struct ubi_attach_info *ai, int pnum, int ec)/** * validate_vid_hdr - check volume identifier header. +* @ubi: UBI device description object * @vid_hdr: the volume identifier header to check * @av: information about the volume this logical eraseblock belongs to * @pnum: physical eraseblock number the VID header came from @@ -48,13 +48,14 @@/** * get_exclusive - get exclusive access to an UBI volume. +* @ubi: UBI device description object * @desc: volume descriptor * * This function changes UBI volume open mode to "exclusive". Returns previous * mode value (positive integer) in case of success and a negative error code * in case of failure. */ @@ -660,13 +660,14 @@ static int init_volumes(struct ubi_device *ubi,/** * check_av - check volume attaching information. +* @ubi: UBI device description object * @vol: UBI volume description object
drivers/mtd/ubi/attach.c static int init_volumes(struct ubi_device *ubi, · line 673
Quoted 10 lines
drivers/mtd/ubi/attach.c +1
-1* @av: volume attaching information * * This function returns zero if the volume attaching information is consistent * to the data read from the volume tabla, and %-EINVAL if not. */ -static int check_av (const struct ubi_volume *vol, +static int check_av (const struct ubi_device *ubi,const struct ubi_volume *vol,const struct ubi_ainf_volume *av) {int err;
This patch add 'struct ubi_device *' for 3 functions. We can get 'ubi_device' from 'ubi_volume'. So I think it's because when we call these functions, the '->ubi' pointer of 'ubi_volume' is not initialized, am I right? This patch use 'vol->ubi' to indicate a 'struct ubi_device *' pointer in some places, I think you are sure of using them.
Quoted 8 lines
drivers/mtd/ubi/build.c +2
-2@@ -1010,28 +1015,28 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ubi->bgt_thread =kthread_create (ubi_thread, ubi,"%s" , ubi->bgt_name);if (IS_ERR (ubi->bgt_thread)) { err =PTR_ERR (ubi->bgt_thread); -ubi_err ("cannot spawn \"%s\", error %d" , ubi->bgt_name, - err); +ubi_err (ubi,"cannot spawn \"%s\", error %d" , + ubi->bgt_name, err);
drivers/mtd/ubi/build.c int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, · line 1026
Quoted 10 lines
drivers/mtd/ubi/build.c +3
-3goto out_debugfs; } -ubi_msg ("attached mtd%d (name \"%s\", size %llu MiB) to ubi%d" , - mtd->index, mtd->name, ubi->flash_size >>20 , ubi_num); -ubi_msg ("PEB size: %d bytes (%d KiB), LEB size: %d bytes" , +ubi_msg (ubi,"attached mtd%d (name \"%s\", size %llu MiB)" , + mtd->index, mtd->name, ubi->flash_size >>20 ); +ubi_msg (ubi,"PEB size: %d bytes (%d KiB), LEB size: %d bytes" , ubi->peb_size, ubi->peb_size >>10 , ubi->leb_size);
We have the parameter 'ubi_num' for log in some functions like 'ubi_attach_mtd_dev' before. This patch remove 'ubi_num' in upper changes but keep it in other changes. Do we have a discussed rule to deal with this situation? It's not a big problem~
Quoted 33 lines
drivers/mtd/ubi/wl.c +6
-3@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err; -if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }if (try_to_freeze ())continue ;@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err;
drivers/mtd/ubi/wl.c int ubi_thread(void *u) · line 1820
Quoted 10 lines
drivers/mtd/ubi/wl.c +4
-1-if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }if (try_to_freeze ())continue ;
Here are two new adding messages. Maybe a separate patch is better? Just a suggestion.
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) · line 1426
Quoted 12 lines
drivers/mtd/ubi/io.c +3
-2@@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len)return 0 ; fail: -ubi_err ("self-check failed for PEB %d" , pnum); -ubi_msg ("hex dump of the %d-%d region" , offset, offset + len); +ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); err = -EINVAL ; error:
Artem, I know you have tried to align the message code in different lines, maybe you can check if you lose this one.
Thanks~!
Hu
Quoted 26 lines · hujianyang
Quoted 22 lines
drivers/mtd/ubi/wl.c +5
-2@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err; -if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }if (try_to_freeze ())continue ;drivers/mtd/ubi/wl.c +0
-0@@ -470,8 +470,11 @@ struct ubi_wl_entry *ubi_wl_get_fm_peb(struct ubi_device *ubi, int anchor) {struct ubi_wl_entry *e =NULL ;
drivers/mtd/ubi/wl.c struct ubi_wl_entry *ubi_wl_get_fm_peb(struct ubi_device *ubi, int anchor) · line 480
Quoted 10 lines
drivers/mtd/ubi/wl.c +4
-1-if (!ubi->free.rb_node || (ubi->free_count - ubi->beb_rsvd_pebs <1 )) +if (!ubi->free.rb_node || (ubi->free_count - ubi->beb_rsvd_pebs <1 )) { +ubi_warn (ubi,"Can't get peb for fastmap:anchor=%d, free_cnt=%d, reserved=%d" , + anchor, ubi->free_count, ubi->beb_rsvd_pebs); goto out; + }if (anchor) e =find_anchor_wl_entry (&ubi->free);
Sorry, one of the adding messages should be this~!
Quoted 26 lines
Quoted 12 lines
drivers/mtd/ubi/wl.c +1
-1@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err;drivers/mtd/ubi/wl.c int ubi_thread(void *u) · line 1820
Quoted 10 lines
drivers/mtd/ubi/wl.c +4
-1-if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }if (try_to_freeze ())continue ;Here are two new adding messages. Maybe a separate patch is better? Just a suggestion.
Quoted 8 lines · hujianyang
drivers/mtd/ubi/build.c static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp) · line 1434
Quoted 5 lines
drivers/mtd/ubi/build.c +1
-1if (len ==0 ) { -pr_warn ("UBI warning: empty 'mtd=' parameter - ignored\n" ); +pr_err ("UBI warning: empty 'mtd=' parameter - ignored\n" );return 0 ; }Why the last '
pr_warn()' need to be changed into 'pr_err()'? I looked up your V1 and V2 patches, I think it's not your purpose.
Well-spotted, thanks.
Quoted 11 lines
drivers/mtd/ubi/vtbl.c static int init_volumes(struct ubi_device *ubi, · line 673
Quoted 5 lines
drivers/mtd/ubi/vtbl.c +1
-1-static int check_av (const struct ubi_volume *vol, +static int check_av (const struct ubi_device *ubi,const struct ubi_volume *vol,const struct ubi_ainf_volume *av) {int err;This patch add 'struct
ubi_device*' for 3 functions. We can get 'ubi_device' from 'ubi_volume'. So I think it's because when we call these functions, the '->ubi' pointer of 'ubi_volume' is not initialized, am I right? This patch use 'vol->ubi' to indicate a 'structubi_device*' pointer in some places, I think you are sure of using them.
Yeah, let's remove the unneeded argument indeed.
Quoted 11 lines
drivers/mtd/ubi/build.c int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, · line 1026
Quoted 7 lines
drivers/mtd/ubi/build.c +3
-3-ubi_msg ("attached mtd%d (name \"%s\", size %llu MiB) to ubi%d" , - mtd->index, mtd->name, ubi->flash_size >>20 , ubi_num); -ubi_msg ("PEB size: %d bytes (%d KiB), LEB size: %d bytes" , +ubi_msg (ubi,"attached mtd%d (name \"%s\", size %llu MiB)" , + mtd->index, mtd->name, ubi->flash_size >>20 ); +ubi_msg (ubi,"PEB size: %d bytes (%d KiB), LEB size: %d bytes" , ubi->peb_size, ubi->peb_size >>10 , ubi->leb_size);We have the parameter '
ubi_num' for log in some functions like 'ubi_attach_mtd_dev' before. This patch remove 'ubi_num' in upper changes but keep it in other changes. Do we have a discussed rule to deal with this situation? It's not a big problem~
Well, printing 'ubi_num' explicitely is not needed anymore, so it would be good to make the code consistent and remove it from other places, where it is not needed.
Quoted 7 lines
Quoted 7 lines
drivers/mtd/ubi/wl.c +4
-1-if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }
Quoted 5 lines
drivers/mtd/ubi/wl.c int ubi_thread(void *u) · line 1820
Quoted 2 lines
drivers/mtd/ubi/wl.c +0
-0if (try_to_freeze ())continue ;Here are two new adding messages. Maybe a separate patch is better? Just a suggestion.
Yes, please, remove these messages or turn them into debugging messages. And yes, these should have been added in a separate patch.
Quoted 15 lines
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) · line 1426
Quoted 12 lines
drivers/mtd/ubi/io.c +3
-2@@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len)return 0 ; fail: -ubi_err ("self-check failed for PEB %d" , pnum); -ubi_msg ("hex dump of the %d-%d region" , offset, offset + len); +ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); err = -EINVAL ; error:Artem, I know you have tried to align the message code in different lines, maybe you can check if you lose this one.
Yeah, lets' correct this too.
Thanks for ferview Hujianyang!
Tanya, would you send a follow-up patch these?
Artem.
Quoted 6 lines · Artem Bityutskiy
Yeah, lets' correct this too.
Thanks for ferview Hujianyang!
Tanya, would you send a follow-up patch these?
Yes, sure.
Sorry, was pulled into some urgent stuff. Will post a new patch early next week.
Quoted 3 lines
Artem.
Thanks,
Tanya Brokhman
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 26 lines · hujianyang
Hi Tanya,
When I was trying to push this patch to my product, I reviewed this patch and found some small problems. I wish it's not too late to report these.
The patch I get from linux-ubifs.git is amended a bit by Artem. I'd like to quote your V5 patch for simplification. Some line numbers may mismatching.
Quoted 18 lines
drivers/mtd/ubi/build.c +2
-2@@ -1408,20 +1416,20 @@ static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp)return -EINVAL ;if (mtd_devs ==UBI_MAX_DEVICES ) { -ubi_err ("too many parameters, max. is %d\n" , +pr_err ("UBI error: too many parameters, max. is %d\n" ,UBI_MAX_DEVICES );return -EINVAL ; } len =strnlen (val,MTD_PARAM_LEN_MAX );if (len ==MTD_PARAM_LEN_MAX ) { -ubi_err ("parameter \"%s\" is too long, max. is %d\n" , +pr_err ("UBI error: parameter \"%s\" is too long, max. is %d\n" , val,MTD_PARAM_LEN_MAX );return -EINVAL ; }
Quoted 8 lines
drivers/mtd/ubi/build.c static int __init ubi_mtd_param_parse(const char *val, struct kernel_param *kp) · line 1434
Quoted 5 lines
drivers/mtd/ubi/build.c +1
-1if (len ==0 ) { -pr_warn ("UBI warning: empty 'mtd=' parameter - ignored\n" ); +pr_err ("UBI warning: empty 'mtd=' parameter - ignored\n" );return 0 ; }Why the last '
pr_warn()' need to be changed into 'pr_err()'? I looked up your V1 and V2 patches, I think it's not your purpose.
It slipped somehow. Thanks! fixed.
Quoted 38 lines
Quoted 38 lines
drivers/mtd/ubi/attach.c +4
-1@@ -176,6 +176,7 @@ static int add_corrupted(struct ubi_attach_info *ai, int pnum, int ec)/** * validate_vid_hdr - check volume identifier header. +* @ubi: UBI device description object * @vid_hdr: the volume identifier header to check * @av: information about the volume this logical eraseblock belongs to * @pnum: physical eraseblock number the VID header came from @@ -48,13 +48,14 @@/** * get_exclusive - get exclusive access to an UBI volume. +* @ubi: UBI device description object * @desc: volume descriptor * * This function changes UBI volume open mode to "exclusive". Returns previous * mode value (positive integer) in case of success and a negative error code * in case of failure. */ @@ -660,13 +660,14 @@ static int init_volumes(struct ubi_device *ubi,/** * check_av - check volume attaching information. +* @ubi: UBI device description object * @vol: UBI volume description object * @av: volume attaching information * * This function returns zero if the volume attaching information is consistent * to the data read from the volume tabla, and %-EINVAL if not. */ -static int check_av (const struct ubi_volume *vol, +static int check_av (const struct ubi_device *ubi,const struct ubi_volume *vol,const struct ubi_ainf_volume *av)
Quoted 9 lines
drivers/mtd/ubi/vtbl.c static int init_volumes(struct ubi_device *ubi, · line 673
Quoted 2 lines
drivers/mtd/ubi/vtbl.c +0
-0{int err;This patch add 'struct
ubi_device*' for 3 functions. We can get 'ubi_device' from 'ubi_volume'. So I think it's because when we call these functions, the '->ubi' pointer of 'ubi_volume' is not initialized, am I right? This patch use 'vol->ubi' to indicate a 'structubi_device*' pointer in some places, I think you are sure of using them.
1. for validate_vid_hdr() we don;t have a ubi_volume yet since its part of the attach process so we need struct ubi_device
2. for get_exclusive() - you're right. Will fetch dev number from the volume
3. for check_av() - you're right. fixed
Quoted 13 lines
Quoted 13 lines
drivers/mtd/ubi/build.c +2
-2@@ -1010,28 +1015,28 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, ubi->bgt_thread =kthread_create (ubi_thread, ubi,"%s" , ubi->bgt_name);if (IS_ERR (ubi->bgt_thread)) { err =PTR_ERR (ubi->bgt_thread); -ubi_err ("cannot spawn \"%s\", error %d" , ubi->bgt_name, - err); +ubi_err (ubi,"cannot spawn \"%s\", error %d" , + ubi->bgt_name, err); goto out_debugfs; }
Quoted 11 lines
drivers/mtd/ubi/build.c int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, · line 1026
Quoted 7 lines
drivers/mtd/ubi/build.c +3
-3-ubi_msg ("attached mtd%d (name \"%s\", size %llu MiB) to ubi%d" , - mtd->index, mtd->name, ubi->flash_size >>20 , ubi_num); -ubi_msg ("PEB size: %d bytes (%d KiB), LEB size: %d bytes" , +ubi_msg (ubi,"attached mtd%d (name \"%s\", size %llu MiB)" , + mtd->index, mtd->name, ubi->flash_size >>20 ); +ubi_msg (ubi,"PEB size: %d bytes (%d KiB), LEB size: %d bytes" , ubi->peb_size, ubi->peb_size >>10 , ubi->leb_size);We have the parameter '
ubi_num' for log in some functions like 'ubi_attach_mtd_dev' before. This patch remove 'ubi_num' in upper changes but keep it in other changes. Do we have a discussed rule to deal with this situation? It's not a big problem~
I removed it because it made no sense printing it twice:
"ubi-0: attached mtd-0 (...) to ubi0"?
so I shortned the message:
"ubi-0: attched mtd..."
All the info is still there....
Same for other messages that printed ubi number.
Quoted 44 lines
Quoted 44 lines
drivers/mtd/ubi/wl.c +10
-4@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err; -if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }if (try_to_freeze ())continue ;@@ -1798,15 +1803,18 @@ int ubi_thread(void *u)int failures =0 ;struct ubi_device *ubi = u; -ubi_msg ("background thread \"%s\" started, PID %d" , +ubi_msg (ubi,"background thread \"%s\" started, PID %d" , ubi->bgt_name,task_pid_nr (current));set_freezable ();for (;;) {int err; -if (kthread_should_stop ()) +if (kthread_should_stop ()) { +ubi_msg (ubi,"background thread \"%s\" should stop, PID %d" , + ubi->bgt_name,task_pid_nr (current));break ; + }
Quoted 5 lines
drivers/mtd/ubi/wl.c int ubi_thread(void *u) · line 1820
Quoted 2 lines
drivers/mtd/ubi/wl.c +0
-0if (try_to_freeze ())continue ;Here are two new adding messages. Maybe a separate patch is better? Just a suggestion.
Done.
Quoted 5 lines
Quoted 5 lines
(quoted diff) +0
-0@@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) return 0;
Quoted 13 lines
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) · line 1426
Quoted 9 lines
drivers/mtd/ubi/io.c +3
-2fail: -ubi_err ("self-check failed for PEB %d" , pnum); -ubi_msg ("hex dump of the %d-%d region" , offset, offset + len); +ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); err = -EINVAL ; error:Artem, I know you have tried to align the message code in different lines, maybe you can check if you lose this one.
hmmm... not sure I understand what is wrong here....
Quoted 7 lines
Thanks~!
Hu
Thanks,
Tanya Brokhman
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Tanya,
Quoted 8 lines · Tanya Brokhman
Quoted 7 lines
This patch add 'struct
ubi_device*' for 3 functions. We can get 'ubi_device' from 'ubi_volume'. So I think it's because when we call these functions, the '->ubi' pointer of 'ubi_volume' is not initialized, am I right? This patch use 'vol->ubi' to indicate a 'structubi_device*' pointer in some places, I think you are sure of using them.
Quoted 4 lines
1. for
validate_vid_hdr()we don;t have aubi_volumeyet since its part of the attach process so we need structubi_device
2. forget_exclusive()- you're right. Will fetch dev number from the volume
3. forcheck_av()- you're right. fixed
I'm not sure if 'ubi_volume->ubi' is initialized when we call some kinds of ubi_err() to print error messages. The reference to a null pointer, we perform 'ubi->ubi_num' in ubi_err(), may crash the kernel. So you should be careful of these situations not only in above cases but also in other places in your patch.
Quoted 5 lines
Quoted 4 lines
We have the parameter '
ubi_num' for log in some functions like 'ubi_attach_mtd_dev' before. This patch remove 'ubi_num' in upper changes but keep it in other changes. Do we have a discussed rule to deal with this situation? It's not a big problem~
Quoted 7 lines
I removed it because it made no sense printing it twice:
"ubi-0: attached mtd-0 (...) to ubi0"?
so I shortned the message:
"ubi-0: attched mtd..."
All the info is still there....
Same for other messages that printed ubi number.
Could we remove some the 'ubi_num'? I think there are no need to print it twice in other places, like:
@@ -921,7 +923,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
/* Make sure ubi_num is not busy */ if (ubi_devices[ubi_num]) {
- ubi_err("ubi%d already exists", ubi_num);
+ ubi_err(ubi, "ubi%d already exists", ubi_num);
return -EEXIST;
}
}
and
@@ -970,7 +974,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
mutex_init(&ubi->fm_mutex);
init_rwsem(&ubi->fm_sem);
- ubi_msg("attaching mtd%d to ubi%d", mtd->index, ubi_num);
+ ubi_msg(ubi, "attaching mtd%d to ubi%d", mtd->index, ubi_num);
Quoted 17 lines
Quoted 16 lines
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) · line 1426
Quoted 12 lines
drivers/mtd/ubi/io.c +3
-2@@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len)return 0 ; fail: -ubi_err ("self-check failed for PEB %d" , pnum); -ubi_msg ("hex dump of the %d-%d region" , offset, offset + len); +ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); err = -EINVAL ; error:Artem, I know you have tried to align the message code in different lines, maybe you can check if you lose this one.
Quoted 2 lines
hmmm... not sure I understand what is wrong here....
Turn
+ ubi_msg(ubi, "hex dump of the %d-%d region",
+ offset, offset + len);
to
+ ubi_msg(ubi, "hex dump of the %d-%d region",
+ offset, offset + len);
Maybe like this. The next line aligns to the message in first line, not a big problem. By the way, I use space in this example, it's wrong. Tab is right.
Thanks!
Hu
Quoted 51 lines · hujianyang
Hi Tanya,
Quoted 8 lines · Tanya Brokhman
Quoted 7 lines
This patch add 'struct
ubi_device*' for 3 functions. We can get 'ubi_device' from 'ubi_volume'. So I think it's because when we call these functions, the '->ubi' pointer of 'ubi_volume' is not initialized, am I right? This patch use 'vol->ubi' to indicate a 'structubi_device*' pointer in some places, I think you are sure of using them.Quoted 4 lines
1. for
validate_vid_hdr()we don;t have aubi_volumeyet since its part of the attach process so we need structubi_device
2. forget_exclusive()- you're right. Will fetch dev number from the volume
3. forcheck_av()- you're right. fixedI'm not sure if '
ubi_volume->ubi' is initialized when we call some kinds ofubi_err()to print error messages. The reference to a null pointer, we perform 'ubi->ubi_num' inubi_err(), may crash the kernel. So you should be careful of these situations not only in above cases but also in other places in your patch.Quoted 5 lines
Quoted 4 lines
We have the parameter '
ubi_num' for log in some functions like 'ubi_attach_mtd_dev' before. This patch remove 'ubi_num' in upper changes but keep it in other changes. Do we have a discussed rule to deal with this situation? It's not a big problem~Quoted 7 lines
I removed it because it made no sense printing it twice:
"ubi-0: attached mtd-0 (...) to ubi0"?
so I shortned the message:
"ubi-0: attched mtd..."
All the info is still there....
Same for other messages that printed ubi number.Could we remove some the '
ubi_num'? I think there are no need to print it twice in other places, like:drivers/mtd/ubi/build.c +1
-1@@ -921,7 +923,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,/* Make sure ubi_num is not busy */ if (ubi_devices[ubi_num]) { -ubi_err ("ubi%d already exists" , ubi_num); +ubi_err (ubi,"ubi%d already exists" , ubi_num);return -EEXIST ; } }and
drivers/mtd/ubi/build.c int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num, · line 979
Quoted 7 lines
drivers/mtd/ubi/build.c +1
-1@@ -970,7 +974,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,mutex_init (&ubi->fm_mutex);init_rwsem (&ubi->fm_sem); -ubi_msg ("attaching mtd%d to ubi%d" , mtd->index, ubi_num); +ubi_msg (ubi,"attaching mtd%d to ubi%d" , mtd->index, ubi_num);
yes, already removed
Quoted 27 lines
(quoted diff) +0
-0>>> @@ -1415,8 +1418,9 @@ int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len)Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
return 0;fail:
drivers/mtd/ubi/io.c +3
-2-ubi_err ("self-check failed for PEB %d" , pnum); -ubi_msg ("hex dump of the %d-%d region" , offset, offset + len); +ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);print_hex_dump (KERN_DEBUG ,"" ,DUMP_PREFIX_OFFSET ,32 ,1 , buf, len,1 ); err = -EINVAL ; error:Quoted 6 lines
Quoted 3 lines
Artem, I know you have tried to align the message code in different lines, maybe you can check if you lose this one.
hmmm... not sure I understand what is wrong here....
Turn
drivers/mtd/ubi/io.c +2
-0+ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);to
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) · line 1423
Quoted 2 lines
drivers/mtd/ubi/io.c +2
-0+ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len);
Actually, I just made it all in one line.... thanks!
Quoted 10 lines
Maybe like this. The next line aligns to the message in first line, not a big problem. By the way, I use space in this example, it's wrong. Tab is right.
Thanks!
Hu
Will upload new version soon. Just running some tests.
Thanks,
Tanya Brokhman
Signature · 3 lines
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 8 lines · Tanya Brokhman
Quoted 4 lines
drivers/mtd/ubi/io.c int ubi_self_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len)
Quoted 4 lines
drivers/mtd/ubi/io.c +3
-0+ubi_err (ubi,"self-check failed for PEB %d" , pnum); +ubi_msg (ubi,"hex dump of the %d-%d region" , + offset, offset + len); print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, buf,len, 1);
Quoted 3 lines
Quoted 2 lines
err = -EINVAL;error:
Quoted 6 lines
Quoted 1 line
Artem, I know you have tried to align the message code in different
lines, maybe
Quoted 2 lines
you can check if you lose this one.
hmmm... not sure I understand what is wrong here....
It is more of a nit-pick, but we try to be consistent. Here is how we align split messages:
ubi_msg(ubi, "blah",
XYZ)
and not
ubi_msg(ubi, "blah",
XYZ)
So we first use few tabs, and then some spaces to align. You use just tabs. Sometimes the second line is aligned, sometimes it goes a bit further.