Re: [PATCH v2 for v3.15 0/3] UBI: block: Support very large volumes

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

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".
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com