All of lore.kernel.org
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Stefani Seibold <stefani@seibold.net>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>,
	"Enzinger,
	Robert \(EXT-Other - DE/Munich\)" <robert.enzinger.ext@nsn.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] Add quick erase format option
Date: Wed, 01 Sep 2010 03:47:23 +0300	[thread overview]
Message-ID: <1283302043.2018.92.camel@brekeke> (raw)
In-Reply-To: <1283236978.6083.28.camel@wall-e.seibold.net>

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.

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

WARNING: multiple messages have this Message-ID (diff)
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Stefani Seibold <stefani@seibold.net>
Cc: David Woodhouse <dwmw2@infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	Artem Bityutskiy <Artem.Bityutskiy@nokia.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"Enzinger,
	Robert (EXT-Other - DE/Munich)"  <robert.enzinger.ext@nsn.com>
Subject: Re: [PATCH] Add quick erase format option
Date: Wed, 01 Sep 2010 03:47:23 +0300	[thread overview]
Message-ID: <1283302043.2018.92.camel@brekeke> (raw)
In-Reply-To: <1283236978.6083.28.camel@wall-e.seibold.net>

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.

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


  reply	other threads:[~2010-09-01  0:47 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-09  8:25 [PATCH] Add quick erase format option stefani
2010-08-09  8:37 ` David Woodhouse
2010-08-09  8:37   ` David Woodhouse
2010-08-09  8:52   ` Stefani Seibold
2010-08-09  8:52     ` Stefani Seibold
2010-08-09 11:29     ` Artem Bityutskiy
2010-08-09 11:29       ` Artem Bityutskiy
2010-08-09 13:37       ` Stefani Seibold
2010-08-09 13:37         ` Stefani Seibold
2010-08-09 13:54       ` Stefani Seibold
2010-08-09 13:54         ` Stefani Seibold
2010-08-29 11:30         ` Artem Bityutskiy
2010-08-29 11:30           ` Artem Bityutskiy
2010-08-29 12:20           ` Artem Bityutskiy
2010-08-29 12:20             ` Artem Bityutskiy
2010-08-31  6:42           ` Stefani Seibold
2010-08-31  6:42             ` Stefani Seibold
2010-09-01  0:47             ` Artem Bityutskiy [this message]
2010-09-01  0:47               ` Artem Bityutskiy
2010-09-02  6:53               ` Stefani Seibold
2010-09-02  6:53                 ` Stefani Seibold
2010-09-02 10:58                 ` Artem Bityutskiy
2010-09-02 10:58                   ` Artem Bityutskiy
2010-09-02 11:42                   ` Stefani Seibold
2010-09-02 11:42                     ` Stefani Seibold
2018-06-20  5:38     ` Richard Weinberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1283302043.2018.92.camel@brekeke \
    --to=dedekind1@gmail.com \
    --cc=Artem.Bityutskiy@nokia.com \
    --cc=akpm@linux-foundation.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=robert.enzinger.ext@nsn.com \
    --cc=stefani@seibold.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.