[PATCH] UBI: Decrease I/O count in ubi_scan

[PATCH] UBI: Decrease I/O count in ubi_scan

[Corentin Chary]

Merge ec_hdr and vid_hdr read in one ubi_io_read()
This result in a 25% speedup with nandsim, and 6%
speedup on an AT91SAM9263 board with a 200Mo UBI volume.

Signed-off-by: Corentin Chary <corentincj@iksaif.net>
---
 drivers/mtd/ubi/io.c   |   67 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mtd/ubi/scan.c |   35 +++++++++++++------------
 drivers/mtd/ubi/ubi.h  |    4 +++
 3 files changed, 87 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 4e7bcb2..cbb5f5b 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -684,7 +684,6 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		       struct ubi_ec_hdr *ec_hdr, int verbose)
 {
 	int err, read_err = 0;
-	uint32_t crc, magic, hdr_crc;
 
 	dbg_io("read EC header from PEB %d", pnum);
 	ubi_assert(pnum >= 0 && pnum < ubi->peb_count);
@@ -705,6 +704,37 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		 */
 		read_err = err;
 	}
+	return ubi_parse_ec_hdr(ubi, pnum, ec_hdr, verbose, read_err);
+}
+
+/**
+ * ubi_parse_ec_hdr - check an erase counter header.
+ * @ubi: UBI device description object
+ * @pnum: physical eraseblock to read from
+ * @ec_hdr: a &struct ubi_ec_hdr object where to store the read erase counter
+ * header
+ * @verbose: be verbose if the header is corrupted or was not found
+ * @read_err: error code returned when reading the header
+ *
+ * This function checks CRC checksum of the read erase counter header.
+ * The following codes may be returned:
+ *
+ * o %0 if the CRC checksum is correct and the header was successfully read;
+ * o %UBI_IO_BITFLIPS if the CRC is correct, but bit-flips were detected
+ *   and corrected by the flash driver; this is harmless but may indicate that
+ *   this eraseblock may become bad soon (but may be not);
+ * o %UBI_IO_BAD_EC_HDR if the erase counter header is corrupted (a CRC error);
+ * o %UBI_IO_PEB_EMPTY if the physical eraseblock is empty;
+ * o a negative error code in case of failure.
+ */
+int ubi_parse_ec_hdr(struct ubi_device *ubi, int pnum,
+		     struct ubi_ec_hdr *ec_hdr, int verbose, int read_err)
+{
+	int err;
+	uint32_t crc, magic, hdr_crc;
+
+	dbg_io("parse EC header from PEB %d", pnum);
+	ubi_assert(pnum >= 0 && pnum < ubi->peb_count);
 
 	magic = be32_to_cpu(ec_hdr->magic);
 	if (magic != UBI_EC_HDR_MAGIC) {
@@ -954,7 +984,6 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 			struct ubi_vid_hdr *vid_hdr, int verbose)
 {
 	int err, read_err = 0;
-	uint32_t crc, magic, hdr_crc;
 	void *p;
 
 	dbg_io("read VID header from PEB %d", pnum);
@@ -978,6 +1007,40 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 		read_err = err;
 	}
 
+	return ubi_parse_vid_hdr(ubi, pnum, vid_hdr, verbose, read_err);
+}
+
+/**
+ * ubi_parse_vid_hdr - check a volume identifier header.
+ * @ubi: UBI device description object
+ * @pnum: physical eraseblock number to read from
+ * @vid_hdr: &struct ubi_vid_hdr object where to store the read volume
+ * identifier header
+ * @verbose: be verbose if the header is corrupted or wasn't found
+ * @read_err: error code returned when reading the header
+ *
+ * This function checks CRC checksum of the read volume identifier header.
+ * The following codes may be returned:
+ *
+ * o %0 if the CRC checksum is correct and the header was successfully read;
+ * o %UBI_IO_BITFLIPS if the CRC is correct, but bit-flips were detected
+ *   and corrected by the flash driver; this is harmless but may indicate that
+ *   this eraseblock may become bad soon;
+ * o %UBI_IO_BAD_VID_HDR if the volume identifier header is corrupted (a CRC
+ *   error detected);
+ * o %UBI_IO_PEB_FREE if the physical eraseblock is free (i.e., there is no VID
+ *   header there);
+ * o a negative error code in case of failure.
+ */
+int ubi_parse_vid_hdr(struct ubi_device *ubi, int pnum,
+		      struct ubi_vid_hdr *vid_hdr, int verbose, int read_err)
+{
+	int err;
+	uint32_t crc, magic, hdr_crc;
+
+	dbg_io("parse VID header from PEB %d", pnum);
+	ubi_assert(pnum >= 0 &&  pnum < ubi->peb_count);
+
 	magic = be32_to_cpu(vid_hdr->magic);
 	if (magic != UBI_VID_HDR_MAGIC) {
 		/*
diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index db4cf04..8b9f990 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -54,6 +54,7 @@ static int paranoid_check_si(struct ubi_device *ubi, struct ubi_scan_info *si);
 /* Temporary variables used during scanning */
 static struct ubi_ec_hdr *ech;
 static struct ubi_vid_hdr *vidh;
+static void *buf;
 
 /**
  * add_to_list - add physical eraseblock to a list.
@@ -719,7 +720,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		      int pnum)
 {
 	long long uninitialized_var(ec);
-	int err, bitflips = 0, vol_id, ec_corr = 0;
+	int read_err, err, bitflips = 0, vol_id, ec_corr = 0;
 
 	dbg_bld("scan PEB %d", pnum);
 
@@ -737,7 +738,12 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		return 0;
 	}
 
-	err = ubi_io_read_ec_hdr(ubi, pnum, ech, 0);
+	read_err = ubi_io_read(ubi, buf, pnum, 0,
+			       ubi->vid_hdr_aloffset + ubi->vid_hdr_alsize);
+	if (read_err < 0)
+		return read_err;
+
+	err = ubi_parse_ec_hdr(ubi, pnum, ech, 0, read_err);
 	if (err < 0)
 		return err;
 	else if (err == UBI_IO_BITFLIPS)
@@ -797,7 +803,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 
 	/* OK, we've done with the EC header, let's look at the VID header */
 
-	err = ubi_io_read_vid_hdr(ubi, pnum, vidh, 0);
+	err = ubi_parse_vid_hdr(ubi, pnum, vidh, 0, read_err);
 	if (err < 0)
 		return err;
 	else if (err == UBI_IO_BITFLIPS)
@@ -901,13 +907,12 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi)
 	si->is_empty = 1;
 
 	err = -ENOMEM;
-	ech = kzalloc(ubi->ec_hdr_alsize, GFP_KERNEL);
-	if (!ech)
+	buf = kzalloc(ubi->vid_hdr_aloffset + ubi->vid_hdr_alsize, GFP_KERNEL);
+	if (!buf)
 		goto out_si;
 
-	vidh = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
-	if (!vidh)
-		goto out_ech;
+	ech = buf;
+	vidh = buf + ubi->vid_hdr_aloffset + ubi->vid_hdr_shift;
 
 	for (pnum = 0; pnum < ubi->peb_count; pnum++) {
 		cond_resched();
@@ -915,7 +920,7 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi)
 		dbg_gen("process PEB %d", pnum);
 		err = process_eb(ubi, si, pnum);
 		if (err < 0)
-			goto out_vidh;
+			goto out_buf;
 	}
 
 	dbg_msg("scanning is finished");
@@ -968,18 +973,14 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi)
 	if (err) {
 		if (err > 0)
 			err = -EINVAL;
-		goto out_vidh;
+		goto out_buf;
 	}
-
-	ubi_free_vid_hdr(ubi, vidh);
-	kfree(ech);
+	kfree(buf);
 
 	return si;
 
-out_vidh:
-	ubi_free_vid_hdr(ubi, vidh);
-out_ech:
-	kfree(ech);
+out_buf:
+	kfree(buf);
 out_si:
 	ubi_scan_destroy_si(si);
 	return ERR_PTR(err);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 6a5fe96..1c83fa6 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -543,10 +543,14 @@ int ubi_io_is_bad(const struct ubi_device *ubi, int pnum);
 int ubi_io_mark_bad(const struct ubi_device *ubi, int pnum);
 int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		       struct ubi_ec_hdr *ec_hdr, int verbose);
+int ubi_parse_ec_hdr(struct ubi_device *ubi, int pnum,
+		     struct ubi_ec_hdr *ec_hdr, int verbose, int read_err);
 int ubi_io_write_ec_hdr(struct ubi_device *ubi, int pnum,
 			struct ubi_ec_hdr *ec_hdr);
 int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 			struct ubi_vid_hdr *vid_hdr, int verbose);
+int ubi_parse_vid_hdr(struct ubi_device *ubi, int pnum,
+		      struct ubi_vid_hdr *vid_hdr, int verbose, int read_err);
 int ubi_io_write_vid_hdr(struct ubi_device *ubi, int pnum,
 			 struct ubi_vid_hdr *vid_hdr);
 
-- 
1.6.3.3

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Artem Bityutskiy]

On Tue, 2009-07-21 at 13:26 +0200, Corentin Chary wrote:
> Merge ec_hdr and vid_hdr read in one ubi_io_read()
> This result in a 25% speedup with nandsim, and 6%
> speedup on an AT91SAM9263 board with a 200Mo UBI volume.
> 
> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
> ---
>  drivers/mtd/ubi/io.c   |   67 ++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/mtd/ubi/scan.c |   35 +++++++++++++------------
>  drivers/mtd/ubi/ubi.h  |    4 +++
>  3 files changed, 87 insertions(+), 19 deletions(-)

Yeah, I did the same optimization some time ago:

http://lists.infradead.org/pipermail/linux-mtd/2008-April/021311.html

Could you please take a look at those patches as well, and if they
are any better, use them or improve yours. I do not remember the
details, to be frank.

No one reported a significant improvement, so I dropped it.
nandsim improvement is not very interesting, but the AT91SAM9263 is.
Which flash is there? And why do you presume is the improvement -
just less calls to MTD or really less I/O? Note MTD usually caches
the last read NAND page, so usually this is not about less I/O.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Corentin Chary]

On Tue, Jul 21, 2009 at 2:14 PM, Artem Bityutskiy<dedekind1@gmail.com> wrote:
> On Tue, 2009-07-21 at 13:26 +0200, Corentin Chary wrote:
>> Merge ec_hdr and vid_hdr read in one ubi_io_read()
>> This result in a 25% speedup with nandsim, and 6%
>> speedup on an AT91SAM9263 board with a 200Mo UBI volume.
>>
>> Signed-off-by: Corentin Chary <corentincj@iksaif.net>
>> ---
>>  drivers/mtd/ubi/io.c   |   67 ++++++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/mtd/ubi/scan.c |   35 +++++++++++++------------
>>  drivers/mtd/ubi/ubi.h  |    4 +++
>>  3 files changed, 87 insertions(+), 19 deletions(-)
>
> Yeah, I did the same optimization some time ago:
>
> http://lists.infradead.org/pipermail/linux-mtd/2008-April/021311.html
>
> Could you please take a look at those patches as well, and if they
> are any better, use them or improve yours. I do not remember the
> details, to be frank.

Didn't see them, they are essentially the same. But I suspect yours are better.

> No one reported a significant improvement, so I dropped it.
> nandsim improvement is not very interesting, but the AT91SAM9263 is.
> Which flash is there?

[   12.544598] NAND device: Manufacturer ID: 0xec, Chip ID: 0x36
(Samsung NAND 64MiB 1,8V 8-bit)
[   12.683697] Scanni
[   12.691395] 0x000000000000-0x000000800000 : "Linux Kernel"
[   12.699687] 0x000000800000-0x000002800000 : "Filesystem"
[   12.707648] 0x000002800000-0x000004000000 : "Free"
[   12.719948] UBI: attaching mtd2 to ubi0
[   12.728005] UBI: physical eraseblock size:   16384 bytes (16 KiB)
[   12.741397] UBI: logical eraseblock size:    15360 bytes
[   12.751721] UBI: smallest flash I/O unit:    512
[   12.760202] UBI: VID header offset:          512 (aligned 512)
[   12.767589] UBI: data offset:                1024
[   13.047943] UBI: attached mtd2 to ubi0
[   13.055450] UBI: MTD device name:            "Free"
[   13.063343] UBI: MTD device size:            24 MiB
[   13.071539] UBI: number of good PEBs:        1536
[   13.084148] UBI: number of bad PEBs:         0
[   13.095572] UBI: max. allowed volumes:       89
[   13.105468] UBI: wear-leveling threshold:    4096
[   13.113178] UBI: number of internal volumes: 1
[   13.129826] UBI: number of user volumes:     1
[   13.140834] UBI: available PEBs:             0
[   13.153634] UBI: total number of reserved PEBs: 1536
[   13.160852] UBI: number of PEBs reserved for bad PEB handling: 15
[   13.167731] UBI: max/mean erase counter: 2/1
[   13.177809] UBI: image sequence number: 0

> And why do you presume is the improvement -
> just less calls to MTD or really less I/O? Note MTD usually caches
> the last read NAND page, so usually this is not about less I/O.

In this case it seems vid and ec headers are not on the same page.
So, if I understand things correctly, there *is* more I/O, and the same thing

The same thing occurs with nandsim, because VID header offset is 2048,
I don't know
if it can happen on real flash.

-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Artem Bityutskiy]

On 07/21/2009 04:42 PM, Corentin Chary wrote:
> [   12.544598] NAND device: Manufacturer ID: 0xec, Chip ID: 0x36
> (Samsung NAND 64MiB 1,8V 8-bit)
> [   12.683697] Scanni
> [   12.691395] 0x000000000000-0x000000800000 : "Linux Kernel"
> [   12.699687] 0x000000800000-0x000002800000 : "Filesystem"
> [   12.707648] 0x000002800000-0x000004000000 : "Free"
> [   12.719948] UBI: attaching mtd2 to ubi0
> [   12.728005] UBI: physical eraseblock size:   16384 bytes (16 KiB)
> [   12.741397] UBI: logical eraseblock size:    15360 bytes
> [   12.751721] UBI: smallest flash I/O unit:    512
> [   12.760202] UBI: VID header offset:          512 (aligned 512)
> [   12.767589] UBI: data offset:                1024
> [   13.047943] UBI: attached mtd2 to ubi0
> [   13.055450] UBI: MTD device name:            "Free"
> [   13.063343] UBI: MTD device size:            24 MiB
> [   13.071539] UBI: number of good PEBs:        1536
> [   13.084148] UBI: number of bad PEBs:         0
> [   13.095572] UBI: max. allowed volumes:       89
> [   13.105468] UBI: wear-leveling threshold:    4096
> [   13.113178] UBI: number of internal volumes: 1
> [   13.129826] UBI: number of user volumes:     1
> [   13.140834] UBI: available PEBs:             0
> [   13.153634] UBI: total number of reserved PEBs: 1536
> [   13.160852] UBI: number of PEBs reserved for bad PEB handling: 15
> [   13.167731] UBI: max/mean erase counter: 2/1
> [   13.177809] UBI: image sequence number: 0

This flash does not look like something which could fit a 200MiB
volume.

>> And why do you presume is the improvement -
>> just less calls to MTD or really less I/O? Note MTD usually caches
>> the last read NAND page, so usually this is not about less I/O.
>
> In this case it seems vid and ec headers are not on the same page.

Strange. May be this is related to how they calculate ECC...
Or just a problem in the driver. Normally flashes like this would
have 256 byte sub-pages and have both headers in the same page.

Could you please check this? I just do not want to optimize it
for a bad/bogus driver. And I think now I recall that this patch
introduced slow down in some cases, when the driver can avoid reading
full page and calculatingfull ECC.

> So, if I understand things correctly, there *is* more I/O, and the same thing

Should not be more I/O.

> The same thing occurs with nandsim, because VID header offset is 2048,
> I don't know
> if it can happen on real flash.

With nandsim you should always have both headers on the same page, unless
you deliberately asked for the opposite by using -s option of ubiformat.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Corentin Chary]

On Tue, Jul 21, 2009 at 4:15 PM, Artem Bityutskiy<dedekind1@gmail.com> wrote:
> On 07/21/2009 04:42 PM, Corentin Chary wrote:
>>
>> [   12.544598] NAND device: Manufacturer ID: 0xec, Chip ID: 0x36
>> (Samsung NAND 64MiB 1,8V 8-bit)
>> [   12.683697] Scanni
>> [   12.691395] 0x000000000000-0x000000800000 : "Linux Kernel"
>> [   12.699687] 0x000000800000-0x000002800000 : "Filesystem"
>> [   12.707648] 0x000002800000-0x000004000000 : "Free"
>> [   12.719948] UBI: attaching mtd2 to ubi0
>> [   12.728005] UBI: physical eraseblock size:   16384 bytes (16 KiB)
>> [   12.741397] UBI: logical eraseblock size:    15360 bytes
>> [   12.751721] UBI: smallest flash I/O unit:    512
>> [   12.760202] UBI: VID header offset:          512 (aligned 512)
>> [   12.767589] UBI: data offset:                1024
>> [   13.047943] UBI: attached mtd2 to ubi0
>> [   13.055450] UBI: MTD device name:            "Free"
>> [   13.063343] UBI: MTD device size:            24 MiB
>> [   13.071539] UBI: number of good PEBs:        1536
>> [   13.084148] UBI: number of bad PEBs:         0
>> [   13.095572] UBI: max. allowed volumes:       89
>> [   13.105468] UBI: wear-leveling threshold:    4096
>> [   13.113178] UBI: number of internal volumes: 1
>> [   13.129826] UBI: number of user volumes:     1
>> [   13.140834] UBI: available PEBs:             0
>> [   13.153634] UBI: total number of reserved PEBs: 1536
>> [   13.160852] UBI: number of PEBs reserved for bad PEB handling: 15
>> [   13.167731] UBI: max/mean erase counter: 2/1
>> [   13.177809] UBI: image sequence number: 0
>
> This flash does not look like something which could fit a 200MiB
> volume.

I didn't do the test myself I only had the dmesg for this test (there
was some gain too).
I'll try to know more.

>>> And why do you presume is the improvement -
>>> just less calls to MTD or really less I/O? Note MTD usually caches
>>> the last read NAND page, so usually this is not about less I/O.
>>
>> In this case it seems vid and ec headers are not on the same page.
>
> Strange. May be this is related to how they calculate ECC...
> Or just a problem in the driver. Normally flashes like this would
> have 256 byte sub-pages and have both headers in the same page.

I'll try to get more information about this flash, and fix the driver
if there is something to fix.

I didn't know that there was a page cache in mtd/nand, but know I
agree that these patch are useless
if both headers can fit in the same page.

Thanks,

-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Corentin Chary]

On Tue, Jul 21, 2009 at 4:15 PM, Artem Bityutskiy<dedekind1@gmail.com> wrote:
> On 07/21/2009 04:42 PM, Corentin Chary wrote:
>>
>> [   12.544598] NAND device: Manufacturer ID: 0xec, Chip ID: 0x36
>> (Samsung NAND 64MiB 1,8V 8-bit)
>> [   12.683697] Scanni
>> [   12.691395] 0x000000000000-0x000000800000 : "Linux Kernel"
>> [   12.699687] 0x000000800000-0x000002800000 : "Filesystem"
>> [   12.707648] 0x000002800000-0x000004000000 : "Free"
>> [   12.719948] UBI: attaching mtd2 to ubi0
>> [   12.728005] UBI: physical eraseblock size:   16384 bytes (16 KiB)
>> [   12.741397] UBI: logical eraseblock size:    15360 bytes
>> [   12.751721] UBI: smallest flash I/O unit:    512
>> [   12.760202] UBI: VID header offset:          512 (aligned 512)
>> [   12.767589] UBI: data offset:                1024
>> [   13.047943] UBI: attached mtd2 to ubi0
>> [   13.055450] UBI: MTD device name:            "Free"
>> [   13.063343] UBI: MTD device size:            24 MiB
>> [   13.071539] UBI: number of good PEBs:        1536
>> [   13.084148] UBI: number of bad PEBs:         0
>> [   13.095572] UBI: max. allowed volumes:       89
>> [   13.105468] UBI: wear-leveling threshold:    4096
>> [   13.113178] UBI: number of internal volumes: 1
>> [   13.129826] UBI: number of user volumes:     1
>> [   13.140834] UBI: available PEBs:             0
>> [   13.153634] UBI: total number of reserved PEBs: 1536
>> [   13.160852] UBI: number of PEBs reserved for bad PEB handling: 15
>> [   13.167731] UBI: max/mean erase counter: 2/1
>> [   13.177809] UBI: image sequence number: 0
>
> This flash does not look like something which could fit a 200MiB
> volume.
>
>>> And why do you presume is the improvement -
>>> just less calls to MTD or really less I/O? Note MTD usually caches
>>> the last read NAND page, so usually this is not about less I/O.
>>
>> In this case it seems vid and ec headers are not on the same page.
>
> Strange. May be this is related to how they calculate ECC...
> Or just a problem in the driver. Normally flashes like this would
> have 256 byte sub-pages and have both headers in the same page.

After reading more code, when HW ECC is enabled, atmel_nand only
handle full page write.


A last question, to be sure,
If we use something like (Samsung K9F2G08UQM) we have writesize = 2048
and subpage = 512.
VID header offset will be 512.
ec_hdr is in the first subpage
vid_hdr is in the second subpage

So, there will two I/O (two subpage read) ?
Reading nand_base.c I believe that the cache is only used when subpage
read is not possible.

Thanks for your time,

-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org

Re: [PATCH] UBI: Decrease I/O count in ubi_scan

[Artem Bityutskiy]

On 07/22/2009 11:51 AM, Corentin Chary wrote:
> On Tue, Jul 21, 2009 at 4:15 PM, Artem Bityutskiy<dedekind1@gmail.com>  wrote:
>> On 07/21/2009 04:42 PM, Corentin Chary wrote:
>>> [   12.544598] NAND device: Manufacturer ID: 0xec, Chip ID: 0x36
>>> (Samsung NAND 64MiB 1,8V 8-bit)
>>> [   12.683697] Scanni
>>> [   12.691395] 0x000000000000-0x000000800000 : "Linux Kernel"
>>> [   12.699687] 0x000000800000-0x000002800000 : "Filesystem"
>>> [   12.707648] 0x000002800000-0x000004000000 : "Free"
>>> [   12.719948] UBI: attaching mtd2 to ubi0
>>> [   12.728005] UBI: physical eraseblock size:   16384 bytes (16 KiB)
>>> [   12.741397] UBI: logical eraseblock size:    15360 bytes
>>> [   12.751721] UBI: smallest flash I/O unit:    512
>>> [   12.760202] UBI: VID header offset:          512 (aligned 512)
>>> [   12.767589] UBI: data offset:                1024
>>> [   13.047943] UBI: attached mtd2 to ubi0
>>> [   13.055450] UBI: MTD device name:            "Free"
>>> [   13.063343] UBI: MTD device size:            24 MiB
>>> [   13.071539] UBI: number of good PEBs:        1536
>>> [   13.084148] UBI: number of bad PEBs:         0
>>> [   13.095572] UBI: max. allowed volumes:       89
>>> [   13.105468] UBI: wear-leveling threshold:    4096
>>> [   13.113178] UBI: number of internal volumes: 1
>>> [   13.129826] UBI: number of user volumes:     1
>>> [   13.140834] UBI: available PEBs:             0
>>> [   13.153634] UBI: total number of reserved PEBs: 1536
>>> [   13.160852] UBI: number of PEBs reserved for bad PEB handling: 15
>>> [   13.167731] UBI: max/mean erase counter: 2/1
>>> [   13.177809] UBI: image sequence number: 0
>> This flash does not look like something which could fit a 200MiB
>> volume.
>>
>>>> And why do you presume is the improvement -
>>>> just less calls to MTD or really less I/O? Note MTD usually caches
>>>> the last read NAND page, so usually this is not about less I/O.
>>> In this case it seems vid and ec headers are not on the same page.
>> Strange. May be this is related to how they calculate ECC...
>> Or just a problem in the driver. Normally flashes like this would
>> have 256 byte sub-pages and have both headers in the same page.
>
> After reading more code, when HW ECC is enabled, atmel_nand only
> handle full page write.

OK, but I do not see why we have more I/O with your patch. It should
only be about doing less calls.

> A last question, to be sure,
> If we use something like (Samsung K9F2G08UQM) we have writesize = 2048
> and subpage = 512.
> VID header offset will be 512.
> ec_hdr is in the first subpage
> vid_hdr is in the second subpage
>
> So, there will two I/O (two subpage read) ?
> Reading nand_base.c I believe that the cache is only used when subpage
> read is not possible.

Yes, I think so. There was a patch from Alexey some time ago which
will make nand_base check ECCs only for the subpages you read, and
ignore the other ones.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)