Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1197891 > unrolled thread

Re: [PATCH v9 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v9 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support Stefan Agner <stefan@agner.ch> - 2015-08-01 02:40 +0200
    Re: [PATCH v9 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support Brian Norris <computersforpeace@gmail.com> - 2015-08-01 04:00 +0200

#1197891 — Re: [PATCH v9 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support

On 2015-08-01 01:47, Brian Norris wrote:
> On Sat, Aug 01, 2015 at 01:35:52AM +0200, Stefan Agner wrote:
>> On 2015-08-01 01:09, Brian Norris wrote:
> 
>> >> +static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
>> >> +				uint8_t *buf, int oob_required, int page)
>> >> +{
>> >> +	int eccsize = chip->ecc.size;
>> >> +	int stat;
>> >> +
>> >> +	vf610_nfc_read_buf(mtd, buf, eccsize);
>> >> +
>> >> +	if (oob_required)
>> >> +		vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
>> >
>> > To fix the bitflips issue above, you'll just want to unconditionally
>> > read the OOB (it's fine to ignore 'oob_required') and...
>> >
>> >> +
>> >> +	stat = vf610_nfc_correct_data(mtd, buf);
>> >
>> > ...pass in chip->oob_poi as a third argument.
>> >
>>
>> Hm, this probably will have an effect on performance, since we usually
>> omit the OOB if not requested.
> 
> You could test :) I don't really like performance claims without tests.
> (I say this because I added the oob_required flag myself, but just for
> functional purposes, not performance. Many drivers got by just fine by
> always copying the OOB data.)

Did the measurement:

As is:
...
[   30.955675] mtd_speedtest: testing eraseblock write speed
[  143.349572] mtd_speedtest: eraseblock write speed is 4641 KiB/s
[  143.355606] mtd_speedtest: testing eraseblock read speed
[  183.816690] mtd_speedtest: eraseblock read speed is 12893 KiB/s
[  185.874702] mtd_speedtest: testing page write speed
[  302.608719] mtd_speedtest: page write speed is 4468 KiB/s
[  302.614229] mtd_speedtest: testing page read speed
[  343.831663] mtd_speedtest: page read speed is 12656 KiB/s
...

Unconditionally read OOB:
...
[   29.076983] mtd_speedtest: testing eraseblock write speed
[  140.829920] mtd_speedtest: eraseblock write speed is 4667 KiB/s
[  140.835960] mtd_speedtest: testing eraseblock read speed
[  181.594498] mtd_speedtest: eraseblock read speed is 12798 KiB/s
[  183.652793] mtd_speedtest: testing page write speed
[  299.772069] mtd_speedtest: page write speed is 4492 KiB/s
[  299.777583] mtd_speedtest: testing page read speed
[  341.283668] mtd_speedtest: page read speed is 12568 KiB/s
...

And with conditional OOB again, reading OOB if required in
vf610_nfc_correct_data.
...
[   29.907147] mtd_speedtest: testing eraseblock write speed
[  141.146171] mtd_speedtest: eraseblock write speed is 4689 KiB/s
[  141.152185] mtd_speedtest: testing eraseblock read speed
[  181.644380] mtd_speedtest: eraseblock read speed is 12883 KiB/s
[  183.703198] mtd_speedtest: testing page write speed
[  299.423179] mtd_speedtest: page write speed is 4507 KiB/s
[  299.428671] mtd_speedtest: testing page read speed
[  340.695925] mtd_speedtest: page read speed is 12640 KiB/s
[  342.747510] mtd_speedtest: testing 2 page write speed
...

The last test is probably pointless since we never read a empty page in
the speedtest. So performance hit is measurable but small (somewhat
below 100KiB/s).

This is with 64 bytes OOB. Since OOB sizes are only getting bigger, I
would rather still consider it... What do you think?

--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1197903

On Sat, Aug 01, 2015 at 02:28:06AM +0200, Stefan Agner wrote:
> Did the measurement:
> 
> As is:
> ...
> [   30.955675] mtd_speedtest: testing eraseblock write speed
> [  143.349572] mtd_speedtest: eraseblock write speed is 4641 KiB/s
> [  143.355606] mtd_speedtest: testing eraseblock read speed
> [  183.816690] mtd_speedtest: eraseblock read speed is 12893 KiB/s
> [  185.874702] mtd_speedtest: testing page write speed
> [  302.608719] mtd_speedtest: page write speed is 4468 KiB/s
> [  302.614229] mtd_speedtest: testing page read speed
> [  343.831663] mtd_speedtest: page read speed is 12656 KiB/s
> ...
> 
> Unconditionally read OOB:
> ...
> [   29.076983] mtd_speedtest: testing eraseblock write speed
> [  140.829920] mtd_speedtest: eraseblock write speed is 4667 KiB/s
> [  140.835960] mtd_speedtest: testing eraseblock read speed
> [  181.594498] mtd_speedtest: eraseblock read speed is 12798 KiB/s
> [  183.652793] mtd_speedtest: testing page write speed
> [  299.772069] mtd_speedtest: page write speed is 4492 KiB/s
> [  299.777583] mtd_speedtest: testing page read speed
> [  341.283668] mtd_speedtest: page read speed is 12568 KiB/s
> ...
> 
> And with conditional OOB again, reading OOB if required in
> vf610_nfc_correct_data.
> ...
> [   29.907147] mtd_speedtest: testing eraseblock write speed
> [  141.146171] mtd_speedtest: eraseblock write speed is 4689 KiB/s
> [  141.152185] mtd_speedtest: testing eraseblock read speed
> [  181.644380] mtd_speedtest: eraseblock read speed is 12883 KiB/s
> [  183.703198] mtd_speedtest: testing page write speed
> [  299.423179] mtd_speedtest: page write speed is 4507 KiB/s
> [  299.428671] mtd_speedtest: testing page read speed
> [  340.695925] mtd_speedtest: page read speed is 12640 KiB/s
> [  342.747510] mtd_speedtest: testing 2 page write speed
> ...
> 
> The last test is probably pointless since we never read a empty page in
> the speedtest. So performance hit is measurable but small (somewhat
> below 100KiB/s).
> 
> This is with 64 bytes OOB. Since OOB sizes are only getting bigger, I
> would rather still consider it... What do you think?

If the code isn't that ugly, then keep the conditional OOB. But I'd
consider that performance difference almost negligible, given the noise
for the write tests is on the order of 40 KB/s.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web