Patchwork Add quick erase format option

login
register
mail settings
Submitter Stefani Seibold
Date Aug. 9, 2010, 8:25 a.m.
Message ID <1281342353-18180-1-git-send-email-stefani@seibold.net>
Download mbox | patch
Permalink /patch/61254/
State New
Headers show

Comments

Stefani Seibold - Aug. 9, 2010, 8:25 a.m.
From: Stefani Seibold <stefani@seibold.net>

This patch add a quick format option which skips erasing of already erased
flash blocks. This is useful for first time production environments where
the flash arrived erased.

Signed-off-by: Stefani Seibold <stefani@seibold.net>
---
 ubi-utils/src/ubiformat.c |   29 ++++++++++++++++++++++++++---
 1 files changed, 26 insertions(+), 3 deletions(-)
David Woodhouse - Aug. 9, 2010, 8:37 a.m.
On Mon, 2010-08-09 at 09:25 +0100, stefani@seibold.net wrote:
> From: Stefani Seibold <stefani@seibold.net>
> 
> This patch add a quick format option which skips erasing of already erased
> flash blocks. This is useful for first time production environments where
> the flash arrived erased.
> 
> Signed-off-by: Stefani Seibold <stefani@seibold.net> 

This scares me, given the lengths we had to go to in JFFS2 to cope with
blocks which *look* like they're erased, but which actually start losing
data as soon as you start writing to them because the erase didn't
complete.
Stefani Seibold - Aug. 9, 2010, 8:52 a.m.
Am Montag, den 09.08.2010, 09:37 +0100 schrieb David Woodhouse:
> On Mon, 2010-08-09 at 09:25 +0100, stefani@seibold.net wrote:
> > From: Stefani Seibold <stefani@seibold.net>
> > 
> > This patch add a quick format option which skips erasing of already erased
> > flash blocks. This is useful for first time production environments where
> > the flash arrived erased.
> > 
> > Signed-off-by: Stefani Seibold <stefani@seibold.net> 
> 
> This scares me, given the lengths we had to go to in JFFS2 to cope with
> blocks which *look* like they're erased, but which actually start losing
> data as soon as you start writing to them because the erase didn't
> complete.
> 

I know the drawback. This is why it is only an option which must be
enabled. And in most use cases there is a subsequent ubimkvol, which
will fail if the flash is not correct initialized.

Flash are normally delivered erased. So this save in our production
environment (Nokia Siemens Networks) about 5 minutes per device (256 MB
NOR CFI Flash).

The old JFFS2 was very fast to install the first time on a flash, it was
only a simple mount of the MTD partition. 

Which the quick format option i have now only a slightly first time
installation overhead compared to JFFS2. Without this option the
overhead is more than 5 minutes.

- Stefani
Artem Bityutskiy - Aug. 9, 2010, 11:29 a.m.
On Mon, 2010-08-09 at 10:52 +0200, Stefani Seibold wrote:
> Am Montag, den 09.08.2010, 09:37 +0100 schrieb David Woodhouse:
> > On Mon, 2010-08-09 at 09:25 +0100, stefani@seibold.net wrote:
> > > From: Stefani Seibold <stefani@seibold.net>
> > > 
> > > This patch add a quick format option which skips erasing of already erased
> > > flash blocks. This is useful for first time production environments where
> > > the flash arrived erased.
> > > 
> > > Signed-off-by: Stefani Seibold <stefani@seibold.net> 
> > 
> > This scares me, given the lengths we had to go to in JFFS2 to cope with
> > blocks which *look* like they're erased, but which actually start losing
> > data as soon as you start writing to them because the erase didn't
> > complete.
> > 
> 
> I know the drawback. This is why it is only an option which must be
> enabled. And in most use cases there is a subsequent ubimkvol, which
> will fail if the flash is not correct initialized.
> 
> Flash are normally delivered erased. So this save in our production
> environment (Nokia Siemens Networks) about 5 minutes per device (256 MB
> NOR CFI Flash).
> 
> The old JFFS2 was very fast to install the first time on a flash, it was
> only a simple mount of the MTD partition. 

Not sure what you do, but both UBI and UBIFS auto-format flash if it is
empty, and attaching empty flash should be very fast.

But yes, the first volume creation ioctl will block until everything is
erased, although this is just an implementation issue and in theory,
fixable.

> Which the quick format option i have now only a slightly first time
> installation overhead compared to JFFS2. Without this option the
> overhead is more than 5 minutes.

Are you flashing an UBI image in production? Then what you can do if you
want to be faster is to flash only the blocks which contain image date,
and leave the rest intact, UBI will erase them and write EC header to
them when you first boot the device.

So I think it is better to add an --pristine-flash option, or something
like this. In this case ubiformat won't erase anything, and will assume
everything is 0xFFed without reading. This should be faster and I think
is better to do.
Stefani Seibold - Aug. 9, 2010, 1:37 p.m.
Am Montag, den 09.08.2010, 14:29 +0300 schrieb Artem Bityutskiy: 
> On Mon, 2010-08-09 at 10:52 +0200, Stefani Seibold wrote:
> > Am Montag, den 09.08.2010, 09:37 +0100 schrieb David Woodhouse:
> > > On Mon, 2010-08-09 at 09:25 +0100, stefani@seibold.net wrote:
> > > > From: Stefani Seibold <stefani@seibold.net>
> > > > 
> > > > This patch add a quick format option which skips erasing of already erased
> > > > flash blocks. This is useful for first time production environments where
> > > > the flash arrived erased.
> > > > 
> > > > Signed-off-by: Stefani Seibold <stefani@seibold.net> 
> > > 
> > > This scares me, given the lengths we had to go to in JFFS2 to cope with
> > > blocks which *look* like they're erased, but which actually start losing
> > > data as soon as you start writing to them because the erase didn't
> > > complete.
> > > 
> > 
> > I know the drawback. This is why it is only an option which must be
> > enabled. And in most use cases there is a subsequent ubimkvol, which
> > will fail if the flash is not correct initialized.
> > 
> > Flash are normally delivered erased. So this save in our production
> > environment (Nokia Siemens Networks) about 5 minutes per device (256 MB
> > NOR CFI Flash).
> > 
> > The old JFFS2 was very fast to install the first time on a flash, it was
> > only a simple mount of the MTD partition. 
> 
> Not sure what you do, but both UBI and UBIFS auto-format flash if it is
> empty, and attaching empty flash should be very fast.
> 

I was never able to mount UBIFS without a previous ubimkvol, despite the
flash is already erased.

> But yes, the first volume creation ioctl will block until everything is
> erased, although this is just an implementation issue and in theory,
> fixable.
> 

Here are my timing results for mounting an empty flash as UBIFS:

ubiattach /dev/ubi_ctrl -m 5 -d 1         -->   2.023 sec
ubimkvol /dev/ubi1 -m -N flash            --> 294.574 sec
mount -t ubifs -o sync ubi1:flash /mnt    -->   0.221 sec

or

ubiformat /dev/mtd5                       --> 299.111 sec
ubiattach /dev/ubi_ctrl -m 5 -d 1         -->   0.129 sec
ubimkvol /dev/ubi1 -m -N flash            -->   1.784 sec
mount -t ubifs -o sync ubi1:flash /mnt    -->   0.220 sec

So there is no real benefit between an empty flash and a formated flash.

And this are the timing results for formating and mounting an empty
flash with my patched ubiformat tool:

ubiformat /dev/mtd5 -E                    -->   5.475 sec
ubiattach /dev/ubi_ctrl -m 5 -d 1         -->   0.130 sec
ubimkvol /dev/ubi1 -m -N flash            -->   1.699 sec
mount -t ubifs -o sync ubi1:flash /mnt    -->   0.220 sec

As you can see this is 296,818 vs. 7,522 or 40 times faster!

But maybe i do something wrong. Could you explain this?

> > Which the quick format option i have now only a slightly first time
> > installation overhead compared to JFFS2. Without this option the
> > overhead is more than 5 minutes.
> 
> Are you flashing an UBI image in production? Then what you can do if you
> want to be faster is to flash only the blocks which contain image date,
> and leave the rest intact, UBI will erase them and write EC header to
> them when you first boot the device.
> 

No, we only initialize the flash, mount the UBIFS it and copy files. 

> So I think it is better to add an --pristine-flash option, or something
> like this. In this case ubiformat won't erase anything, and will assume
> everything is 0xFFed without reading. This should be faster and I think
> is better to do.
> 

My patch assumes nothing, it check if the EC header is 0xff and i think
this is safer than your suggestion. My patch skips the erase if all
bytes in the header are 0xff skip, otherwise erase it.

- Stefani
Stefani Seibold - Aug. 9, 2010, 1:54 p.m.
Am Montag, den 09.08.2010, 14:29 +0300 schrieb Artem Bityutskiy: 
> On Mon, 2010-08-09 at 10:52 +0200, Stefani Seibold wrote:
> > Am Montag, den 09.08.2010, 09:37 +0100 schrieb David Woodhouse:
> > > On Mon, 2010-08-09 at 09:25 +0100, stefani@seibold.net wrote:
> > > > From: Stefani Seibold <stefani@seibold.net>
> > > > 
> > > > This patch add a quick format option which skips erasing of already erased
> > > > flash blocks. This is useful for first time production environments where
> > > > the flash arrived erased.
> > > > 
> > > > Signed-off-by: Stefani Seibold <stefani@seibold.net> 
> > > 
> > > This scares me, given the lengths we had to go to in JFFS2 to cope with
> > > blocks which *look* like they're erased, but which actually start losing
> > > data as soon as you start writing to them because the erase didn't
> > > complete.
> > > 
> > 
> > I know the drawback. This is why it is only an option which must be
> > enabled. And in most use cases there is a subsequent ubimkvol, which
> > will fail if the flash is not correct initialized.
> > 
> > Flash are normally delivered erased. So this save in our production
> > environment (Nokia Siemens Networks) about 5 minutes per device (256 MB
> > NOR CFI Flash).
> > 
> > The old JFFS2 was very fast to install the first time on a flash, it was
> > only a simple mount of the MTD partition. 
> 
> Not sure what you do, but both UBI and UBIFS auto-format flash if it is
> empty, and attaching empty flash should be very fast.
> 

I was never able to mount a UBIFS without a previous ubimkvol, despite
the flash is already erased.

Here are my timing results mounting an already erased flash as UBIFS:

ubiattach /dev/ubi_ctrl -m 5 -d 1              -->   2.023s
ubimkvol /dev/ubi1 -m -N flash                 --> 294.574s
mount -t ubifs -o sync ubi1:flash /mnt         -->   0.221s

And this are the timing results when i do an ubiformat first:

ubiformat /dev/mtd5                            --> 299.111s
ubiattach /dev/ubi_ctrl -m 5 -d 1              -->   0.129s
ubimkvol /dev/ubi1 -m -N flash                 -->   1.784s
mount -t ubifs -o sync ubi1:flash /mnt         -->   0.220s

And this are the results with my patched version of the ubiformat tool
using an already erased flash:

ubiformat /dev/mtd5 -E                         -->   5.475s
ubiattach /dev/ubi_ctrl -m 5 -d                -->   0.130s
ubimkvol /dev/ubi1 -m -N flash                 -->   1.699s
mount -t ubifs -o sync ubi1:flash /mnt         -->   0.220s

As you can see this is 296.818s vs. 7.524 or 40 times faster.

Maybe i do something wrong, but i have no idea. Can u explain it to me?

BTW: The flash is a 128 MB CFI AMD NOR and the size of the mtd5
partition is 47 MB.

> But yes, the first volume creation ioctl will block until everything is
> erased, although this is just an implementation issue and in theory,
> fixable.
> 
> > Which the quick format option i have now only a slightly first time
> > installation overhead compared to JFFS2. Without this option the
> > overhead is more than 5 minutes.
> 
> Are you flashing an UBI image in production? Then what you can do if you
> want to be faster is to flash only the blocks which contain image date,
> and leave the rest intact, UBI will erase them and write EC header to
> them when you first boot the device.
> 

No, we only initialize the flash, mount it as UBIFS and copy files.

> So I think it is better to add an --pristine-flash option, or something
> like this. In this case ubiformat won't erase anything, and will assume
> everything is 0xFFed without reading. This should be faster and I think
> is better to do.
> 

This patch assumes nothing, it will skip the erase of the PEB if all
bytes in the EC header are 0xff. I think this is safer than your
suggestion.

- Stefani
Artem Bityutskiy - Aug. 29, 2010, 11:30 a.m.
On Mon, 2010-08-09 at 15:54 +0200, Stefani Seibold wrote:
> > Not sure what you do, but both UBI and UBIFS auto-format flash if it is
> > empty, and attaching empty flash should be very fast.
> > 
> 
> I was never able to mount a UBIFS without a previous ubimkvol, despite
> the flash is already erased.
> 
> Here are my timing results mounting an already erased flash as UBIFS:
> 
> ubiattach /dev/ubi_ctrl -m 5 -d 1              -->   2.023s

UBI scans the flash and schedules all PEBs for erasure. They will be
erased by the back ground thread.

So after this command returns, UBI is still erasing your flash.

> ubimkvol /dev/ubi1 -m -N flash                 --> 294.574s

this calls the mkvol ioctl, which will first flush the erase queue, and
only after it is flushed (all PEBs scheduled for erasure are erased), it
will create the volume.

The reason why this flushing is needed is that we want to make sure that
there are no PEBs which belonged to a volume with the same ID. Indeed,
imaging you have volume 5 with many used LEBs. Then you remove this
volume, so that all its LEBs are scheduled for erasure. Then you create
a new volume with the same ID = 5. If we do not flush the queue, we may
end up with rubbish from the older volume 5 in the new one.

However, we can optimize UBI and make ubimkvol to not flush empty PEBs,
because they anyway do not contain any data. In this case your ubimkvol
will be very fast. This should not be too difficult to do.

> mount -t ubifs -o sync ubi1:flash /mnt         -->   0.221s
> 
> And this are the timing results when i do an ubiformat first:
> 
> ubiformat /dev/mtd5                            --> 299.111s

Right, ubiformat will erase all PEBs, and this is slow.

> ubiattach /dev/ubi_ctrl -m 5 -d 1              -->   0.129s
> ubimkvol /dev/ubi1 -m -N flash                 -->   1.784s
> mount -t ubifs -o sync ubi1:flash /mnt         -->   0.220s
> 
> And this are the results with my patched version of the ubiformat tool
> using an already erased flash:
> 
> ubiformat /dev/mtd5 -E                         -->   5.475s
> ubiattach /dev/ubi_ctrl -m 5 -d                -->   0.130s
> ubimkvol /dev/ubi1 -m -N flash                 -->   1.699s
> mount -t ubifs -o sync ubi1:flash /mnt         -->   0.220s

Right, with your patch you do not erase.

> As you can see this is 296.818s vs. 7.524 or 40 times faster.
> 
> Maybe i do something wrong, but i have no idea. Can u explain it to me?

No, it is ok. The only thing I do not really like is the name of the
option and the fact that you still read the beginning of PEBs and check
for 0xFFs. And if you hit a PEB with non-0xFF, you erase it, for other
PEBs you skip the erasure - this is error prone, because if you have one
PEB with non-FF data, then you may have PEBs with 0xFFs at the beginning
and garbage at the end, and you will not catch these.

I would like to change the option name so that it would reflect the
exact use-case we are creating it for: wiped out flash. So I'd be
happier with something like --pristine-flash.

Also, I think you should not read the beginning of the flash and check
for 0xFFs. If the user gave us this option, he knows what he is doing
and we can trust him, so no need to read and check, assume all PEBs are
pristine (contain 0xFFs).

> No, we only initialize the flash, mount it as UBIFS and copy files.

OK. But did you consider to pre-create the image with ubinize and
mkfs.ubifs tools and just flash the raw image in production? This is the
fastest possible way.

> > So I think it is better to add an --pristine-flash option, or something
> > like this. In this case ubiformat won't erase anything, and will assume
> > everything is 0xFFed without reading. This should be faster and I think
> > is better to do.
> > 
> 
> This patch assumes nothing, it will skip the erase of the PEB if all
> bytes in the EC header are 0xff. I think this is safer than your
> suggestion.

It does assume that if the beginning of the flash contains 0xFFs then it
is safe to treat it as erased. Instead, I think you should just trust
the user and not even check the beginning of the flash. And this will be
also faster.
Artem Bityutskiy - Aug. 29, 2010, 12:20 p.m.
On Sun, 2010-08-29 at 14:30 +0300, Artem Bityutskiy wrote:
> The reason why this flushing is needed is that we want to make sure
> that
> there are no PEBs which belonged to a volume with the same ID. Indeed,
> imaging you have volume 5 with many used LEBs. Then you remove this
> volume, so that all its LEBs are scheduled for erasure. Then you
> create
> a new volume with the same ID = 5. If we do not flush the queue, we
> may
> end up with rubbish from the older volume 5 in the new one. 

Err, it is important to say that we'll end up with the rubbish if we
have an unclean reboot after the new volume with ID 5 is created, but
before all PEBs belonging to the old volume with ID 5 are erased.
Hopefully, my explanation is not too messy :-)
Stefani Seibold - Aug. 31, 2010, 6:42 a.m.
Am Sonntag, den 29.08.2010, 14:30 +0300 schrieb Artem Bityutskiy:
> On Mon, 2010-08-09 at 15:54 +0200, Stefani Seibold wrote:

> > As you can see this is 296.818s vs. 7.524 or 40 times faster.
> > 
> > Maybe i do something wrong, but i have no idea. Can u explain it to me?
> 
> No, it is ok. The only thing I do not really like is the name of the
> option and the fact that you still read the beginning of PEBs and check
> for 0xFFs. And if you hit a PEB with non-0xFF, you erase it, for other
> PEBs you skip the erasure - this is error prone, because if you have one
> PEB with non-FF data, then you may have PEBs with 0xFFs at the beginning
> and garbage at the end, and you will not catch these.
> 
> I would like to change the option name so that it would reflect the
> exact use-case we are creating it for: wiped out flash. So I'd be
> happier with something like --pristine-flash.

"Pristine" is not a word which non native speaker have in its
vocabulary. Quick-format is the best, because it is exactly what it is
doing. But if you prefer this, you are okay be me.

> 
> Also, I think you should not read the beginning of the flash and check
> for 0xFFs. If the user gave us this option, he knows what he is doing
> and we can trust him, so no need to read and check, assume all PEBs are
> pristine (contain 0xFFs).
> 

The user never knows what he is doing, believe me that is what live
teach me. This is a kind of defensive programming.

> > No, we only initialize the flash, mount it as UBIFS and copy files.
> 
> OK. But did you consider to pre-create the image with ubinize and
> mkfs.ubifs tools and just flash the raw image in production? This is the
> fastest possible way.
> 

This did not work in our NSN transport environment. It would take to
much time to explain why, because the PCU software managment server is a
10 year old application which handles a wide range of transport boards
in the same way, including the old JFFS2 systems and the new UBIFS based
boards.
 
> > > So I think it is better to add an --pristine-flash option, or something
> > > like this. In this case ubiformat won't erase anything, and will assume
> > > everything is 0xFFed without reading. This should be faster and I think
> > > is better to do.
> > > 
> > 
> > This patch assumes nothing, it will skip the erase of the PEB if all
> > bytes in the EC header are 0xff. I think this is safer than your
> > suggestion.
> 
> It does assume that if the beginning of the flash contains 0xFFs then it
> is safe to treat it as erased. Instead, I think you should just trust
> the user and not even check the beginning of the flash. And this will be
> also faster.
> 

Never trust the user. And why should we remove this check? The coast is
very minimal and it will make live much easier.

For example: In our production environment everything is automated by
scripts, so the software bring up did not know if the flash is already
erased or not. It is possible that an broken used board is returned into
the production after it was repaired.

What you assume is that the user or the scripts does know the status of
the flash, but this is not true in real production environment, where
thousands of boards are prepared.

Stefani
Artem Bityutskiy - Sept. 1, 2010, 12:47 a.m.
Stefani,

you have strong points, but I'm still not entirely convinced.

On Tue, 2010-08-31 at 08:42 +0200, Stefani Seibold wrote:
> > I would like to change the option name so that it would reflect the
> > exact use-case we are creating it for: wiped out flash. So I'd be
> > happier with something like --pristine-flash.
> 
> "Pristine" is not a word which non native speaker have in its
> vocabulary.

Agree, not very simple word, when I met it in JFFS2 sources, I looked it
up in the dictionary :-)

>  Quick-format is the best, because it is exactly what it is
> doing.

No, it is misleading. If I was a dumb user, I'd be thinking: oh, why I
should do slow format if I can do quick? It is probably like my FAT32
partition formatting in Windows, I always shoos quick format.

This is why I do not like your naming - it encourages to use it, while
my name discourages.

Also, your use-case is new/virgin/pristine/whatever NOR chips, which are
guaranteed to be erased, and I wanted to use option name which reflects
your use-case.

How about:

--virgin
--factory
--brand-new
--all-erased

Or --do-not-use-me :-)

> > OK. But did you consider to pre-create the image with ubinize and
> > mkfs.ubifs tools and just flash the raw image in production? This is the
> > fastest possible way.
> > 
> 
> This did not work in our NSN transport environment. It would take to
> much time to explain why, because the PCU software managment server is a
> 10 year old application which handles a wide range of transport boards
> in the same way, including the old JFFS2 systems and the new UBIFS based
> boards.

OK :-)

> > It does assume that if the beginning of the flash contains 0xFFs then it
> > is safe to treat it as erased. Instead, I think you should just trust
> > the user and not even check the beginning of the flash. And this will be
> > also faster.
> > 
> 
> Never trust the user. And why should we remove this check? The coast is
> very minimal and it will make live much easier.

Well, this is a good principle, no doubts. But on NAND, it will hurt
performance, because we'll end up with reading whole page. And since
NAND erase is ultra-fast, comparing to NOR, reading whole page will
introduce a noticeable overhead.

IOW, as the maintainer, I have to care about code in general, not only
specific use-cases.

How about checking only the first and last eraseblocks? It would give
_some_ sanity check at least, with less overhead?

Or, do the check only if this is NOR, but I'm less happy about this
approach.

> For example: In our production environment everything is automated by
> scripts, so the software bring up did not know if the flash is already
> erased or not. It is possible that an broken used board is returned into
> the production after it was repaired.

Well, you can check the flash before running ubiformat.

> What you assume is that the user or the scripts does know the status of
> the flash, but this is not true in real production environment, where
> thousands of boards are prepared.

Again, I do not mind to add checks if they are cheap in general, but
this one is not cheap for NAND.
Stefani Seibold - Sept. 2, 2010, 6:53 a.m.
Am Mittwoch, den 01.09.2010, 03:47 +0300 schrieb Artem Bityutskiy:
> Stefani,
> 
> you have strong points, but I'm still not entirely convinced.
> 
> On Tue, 2010-08-31 at 08:42 +0200, Stefani Seibold wrote:
> > > I would like to change the option name so that it would reflect the
> > > exact use-case we are creating it for: wiped out flash. So I'd be
> > > happier with something like --pristine-flash.
> > 
> > "Pristine" is not a word which non native speaker have in its
> > vocabulary.
> 
> Agree, not very simple word, when I met it in JFFS2 sources, I looked it
> up in the dictionary :-)
> 

> Also, your use-case is new/virgin/pristine/whatever NOR chips, which are
> guaranteed to be erased, and I wanted to use option name which reflects
> your use-case.
> 
> How about:
> 
> --virgin
> --factory
> --brand-new
> --all-erased
> 
> Or  :-)

--do-not-use-me is the best. But more seriously i think we should it
split it into two options. --all-erased and --check-erased. The first
assumes that all PEB are erased, while the second do the check if the
PEB is erased and if not it will be erased.

So we can handle NAND's, which have a fast erase, and NOR's  which are
very slow. With this we are able to pick the best option for the
manufacturing.

- Stefani
Artem Bityutskiy - Sept. 2, 2010, 10:58 a.m.
On Thu, 2010-09-02 at 08:53 +0200, Stefani Seibold wrote: 
> --do-not-use-me is the best. But more seriously i think we should it
> split it into two options. --all-erased and --check-erased. The first
> assumes that all PEB are erased, while the second do the check if the
> PEB is erased and if not it will be erased.
> 
> So we can handle NAND's, which have a fast erase, and NOR's  which are
> very slow. With this we are able to pick the best option for the
> manufacturing.

I am fine with checking, but what bothers me is that you check only 64
bytes out of 128KiB - why this is enough to make sure the eraseblock is
erased?

Probably it is ok for you, but in for general use-case this is wrong,
even checking all 128KiB is wrong, because of the unstable bits.

What I think will make more sense is to add general option --verify or
something like that. It would read everything the utility wrote and
verify it is identical to what was written. Probably this can be done in
libmtd.

Then you will be able to combine --all-erased with --verify and achieve
what you want.
Stefani Seibold - Sept. 2, 2010, 11:42 a.m.
Am Donnerstag, den 02.09.2010, 13:58 +0300 schrieb Artem Bityutskiy:
> On Thu, 2010-09-02 at 08:53 +0200, Stefani Seibold wrote: 
> > --do-not-use-me is the best. But more seriously i think we should it
> > split it into two options. --all-erased and --check-erased. The first
> > assumes that all PEB are erased, while the second do the check if the
> > PEB is erased and if not it will be erased.
> > 
> > So we can handle NAND's, which have a fast erase, and NOR's  which are
> > very slow. With this we are able to pick the best option for the
> > manufacturing.
> 
> I am fine with checking, but what bothers me is that you check only 64
> bytes out of 128KiB - why this is enough to make sure the eraseblock is
> erased?
> 
> Probably it is ok for you, but in for general use-case this is wrong,
> even checking all 128KiB is wrong, because of the unstable bits.
> 
> What I think will make more sense is to add general option --verify or
> something like that. It would read everything the utility wrote and
> verify it is identical to what was written. Probably this can be done in
> libmtd.
> 
> Then you will be able to combine --all-erased with --verify and achieve
> what you want.
> 

Agree. I will create a patch for this in the next few days.

- Stefani

Patch

diff --git a/ubi-utils/src/ubiformat.c b/ubi-utils/src/ubiformat.c
index 4e27e4f..8e33334 100644
--- a/ubi-utils/src/ubiformat.c
+++ b/ubi-utils/src/ubiformat.c
@@ -45,7 +45,7 @@ 
 #include "common.h"
 #include "ubiutils-common.h"
 
-#define PROGRAM_VERSION "1.5"
+#define PROGRAM_VERSION "1.6"
 #define PROGRAM_NAME    "ubiformat"
 
 /* The variables below are set by command line arguments */
@@ -55,6 +55,7 @@  struct args {
 	unsigned int verbose:1;
 	unsigned int override_ec:1;
 	unsigned int novtbl:1;
+	unsigned int quick:1;
 	unsigned int manual_subpage;
 	int subpage_size;
 	int vid_hdr_offs;
@@ -94,6 +95,7 @@  static const char *optionsstr =
 "                             (default is 1)\n"
 "-Q, --image-seq=<num>        32-bit UBI image sequence number to use\n"
 "                             (by default a random number is picked)\n"
+"-E, --erase-quick            erase only used blocks\n"
 "-y, --yes                    assume the answer is \"yes\" for all question\n"
 "                             this program would otherwise ask\n"
 "-q, --quiet                  suppress progress percentage information\n"
@@ -125,6 +127,7 @@  static const struct option long_options[] = {
 	{ .name = "ubi-ver",         .has_arg = 1, .flag = NULL, .val = 'x' },
 	{ .name = "help",            .has_arg = 0, .flag = NULL, .val = 'h' },
 	{ .name = "version",         .has_arg = 0, .flag = NULL, .val = 'V' },
+	{ .name = "erase-quick",     .has_arg = 0, .flag = NULL, .val = 'E' },
 	{ NULL, 0, NULL, 0},
 };
 
@@ -138,7 +141,7 @@  static int parse_opt(int argc, char * const argv[])
 		char *endp;
 		unsigned long int image_seq;
 
-		key = getopt_long(argc, argv, "nh?Vyqve:x:s:O:f:S:", long_options, NULL);
+		key = getopt_long(argc, argv, "nh?Vyqve:x:s:O:f:S:E", long_options, NULL);
 		if (key == -1)
 			break;
 
@@ -202,6 +205,10 @@  static int parse_opt(int argc, char * const argv[])
 			break;
 
 
+		case 'E':
+			args.quick = 1;
+			break;
+
 		case 'v':
 			args.verbose = 1;
 			break;
@@ -600,7 +607,20 @@  static int format(libmtd_t libmtd, const struct mtd_dev_info *mtd,
 			ec = si->ec[eb] + 1;
 		else
 			ec = si->mean_ec;
-		ubigen_init_ec_hdr(ui, hdr, ec);
+
+		if (args.quick) {
+			err = mtd_read(mtd, args.node_fd, eb, 0, hdr, write_size);
+			if (!err) {
+				int i;
+
+				for(i = 0; i != write_size; i++) {
+					if (((unsigned char *)hdr)[i] != 0xff)
+						break;
+				}
+				if (i == write_size)
+					goto skip_erase;
+			}
+		}
 
 		if (args.verbose) {
 			normsg_cont("eraseblock %d: erase", eb);
@@ -621,6 +641,9 @@  static int format(libmtd_t libmtd, const struct mtd_dev_info *mtd,
 			continue;
 		}
 
+skip_erase:
+		ubigen_init_ec_hdr(ui, hdr, ec);
+
 		if ((eb1 == -1 || eb2 == -1) && !novtbl) {
 			if (eb1 == -1) {
 				eb1 = eb;