Re: [PATCH v2] UBI: only read necessary size when reading the VID header

[Artem Bityutskiy <dedekind1@gmail.com>]

On Tue, 2016-06-28 at 16:05 +0200, Sascha Hauer wrote:
> On Tue, Jun 28, 2016 at 04:00:29PM +0300, Artem Bityutskiy wrote:
> > On Tue, 2016-06-28 at 13:51 +0200, Sascha Hauer wrote:
> > > When reading the vid hdr from the device UBI always reads a whole
> > > page. Instead, read only the data we actually need and speed up
> > > attachment of UBI devices by potentially making use of reading
> > > subpages if the NAND driver supports it.
> > > 
> > > Since the VID header may be at offset vid_hdr_shift in the page
> > > and
> > > we can only read from the beginning of a page we have to add that
> > > offset to the read size.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > > 
> > > change since v1:
> > > - properly handle vid_hdr_shift != 0
> > > 
> > >  drivers/mtd/ubi/io.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
> > > index 10cf3b5..ff8cafe 100644
> > > --- a/drivers/mtd/ubi/io.c
> > > +++ b/drivers/mtd/ubi/io.c
> > > @@ -1019,7 +1019,7 @@ int ubi_io_read_vid_hdr(struct ubi_device
> > > *ubi,
> > > int pnum,
> > >  
> > >  	p = (char *)vid_hdr - ubi->vid_hdr_shift;
> > >  	read_err = ubi_io_read(ubi, p, pnum, ubi-
> > > >vid_hdr_aloffset,
> > > -			  ubi->vid_hdr_alsize);
> > > +			  ubi->vid_hdr_shift +
> > > UBI_VID_HDR_SIZE);
> > 
> > There are pages and subpages underneath. Let me use old sizes. Say,
> > 2KiB pages, and 512KiB subpages.
> > 
> > If the VID header is in the first subpage, the MTD level probably
> > needs
> > to read the entire subpage anyway, because of per-subpage ECC. This
> > is
> > why we give it a buffer of subpage size.
> 
> The gpmi NAND driver allows reading subpages (NAND_SUBPAGE_READ is
> set), but
> it does not allow writing subpages (NAND_NO_SUBPAGE_WRITE is also
> set).

OK. In the generic code (nand_base.c) though you can find that if the
read request is for entire page or subpage, then the data is transfared
from the chip to the supplied buffer, ECC is verified, and the data are
returned. If the read request is for a fraction of a page or subpage,
then the data are transferred from the chip to an internal buffer, ECC
is verified, and then the required piece of data are copied from the
internal buffer to the supplied buffer. I.e., more memory copy
operations.

The code is very twisted, but I think the logic is in
'nand_do_read_ops()', you'll notice that in the "not aligned" case the
special 'chip->buffers->databuf' is used, otherwise the buffer supplied
by UBI is used.