Message ID | 20140725231001.GA29798@arch.cereza |
---|---|
State | Changes Requested |
Headers | show |
On Fri, 2014-07-25 at 20:10 -0300, Ezequiel Garcia wrote: > I think this series got lost :-( > > It's not in today's -next, and I'm looking at your pull for v3.16 and it's > not there either. It seems I overlooked it, and realised just now, just before > sending a new fix. > > Any idea what happened? Shame, but no idea. May be I applied them on a laptop, and forgot to push, and then never noticed. Anyway, sorry, just applied and pushed out. > return 0; > @@ -412,7 +412,7 @@ int ubiblock_create(struct ubi_volume_info *vi) > gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id; > gd->private_data = dev; > sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id); > - disk_capacity = (vi->size * vi->usable_leb_size) >> 9; > + disk_capacity = vi->used_bytes >> 9; I think you should first align up and then divide. Something like disk_capacity = ALIGN(vi->used_bytes, 512) / 512 may be? By the way, do you really need to shift instead of just dividing, which is more readable. I think with nowadays' compilers ">> 9" and "/ 512" will give the same code, and the latter seems a bit more readable, no?
On 28 Jul 07:05 PM, Artem Bityutskiy wrote: > On Fri, 2014-07-25 at 20:10 -0300, Ezequiel Garcia wrote: > > I think this series got lost :-( > > > > It's not in today's -next, and I'm looking at your pull for v3.16 and it's > > not there either. It seems I overlooked it, and realised just now, just before > > sending a new fix. > > > > Any idea what happened? > > Shame, but no idea. May be I applied them on a laptop, and forgot to > push, and then never noticed. Anyway, sorry, just applied and pushed > out. > Cool, no problem. > > return 0; > > @@ -412,7 +412,7 @@ int ubiblock_create(struct ubi_volume_info *vi) > > gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id; > > gd->private_data = dev; > > sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id); > > - disk_capacity = (vi->size * vi->usable_leb_size) >> 9; > > + disk_capacity = vi->used_bytes >> 9; > > I think you should first align up and then divide. Something like > > disk_capacity = ALIGN(vi->used_bytes, 512) / 512 > > may be? > Yeah, of course. > By the way, do you really need to shift instead of just dividing, which > is more readable. I think with nowadays' compilers ">> 9" and "/ 512" > will give the same code, and the latter seems a bit more readable, no? > Probably. I'm not sure I was thinking in any compiler optimization, but rather trying to follow current practice in other block code: $ git grep ">> 9" block/ drivers/block/ | wc -l 133 $ git grep "/ 512" block/ drivers/block/ | wc -l 12 It sounds like ">> 9" is a frequent enough idiom for "block sector translation".
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c index 8457df7..295a234 100644 --- a/drivers/mtd/ubi/block.c +++ b/drivers/mtd/ubi/block.c @@ -188,8 +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 ubi_read error %d", - dev->gd->disk_name, ret); + ubi_err("%s ubi_read error %d on LEB=%d, off=%d, len=%d", + dev->gd->disk_name, ret, leb, offset, len); return ret; } return 0; @@ -412,7 +412,7 @@ int ubiblock_create(struct ubi_volume_info *vi) gd->first_minor = dev->ubi_num * UBI_MAX_VOLUMES + dev->vol_id; gd->private_data = dev; sprintf(gd->disk_name, "ubiblock%d_%d", dev->ubi_num, dev->vol_id); - disk_capacity = (vi->size * vi->usable_leb_size) >> 9; + disk_capacity = vi->used_bytes >> 9; set_capacity(gd, disk_capacity); dev->gd = gd; @@ -516,9 +516,9 @@ static void ubiblock_resize(struct ubi_volume_info *vi) } mutex_lock(&dev->dev_mutex); - disk_capacity = (vi->size * vi->usable_leb_size) >> 9; + disk_capacity = vi->used_bytes >> 9; set_capacity(dev->gd, disk_capacity); - ubi_msg("%s resized to %d LEBs", dev->gd->disk_name, vi->size); + ubi_msg("%s resized to %d bytes", dev->gd->disk_name, vi->used_bytes); mutex_unlock(&dev->dev_mutex); mutex_unlock(&devices_mutex); }