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

Groups > linux.kernel > #1612082 > unrolled thread

[RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Jesper Nilsson <jesper.nilsson@axis.com> - 2017-03-29 17:50 +0200
    Re: [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Richard Weinberger <richard@nod.at> - 2017-03-29 22:10 +0200
      Re: [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Marek Vasut <marek.vasut@gmail.com> - 2017-03-30 12:10 +0200
        Re: [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Jesper Nilsson <jesper.nilsson@axis.com> - 2017-03-30 19:50 +0200
          Re: [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Richard Weinberger <richard@nod.at> - 2017-03-30 23:30 +0200
            [PATCH v2] UBI: Make MTD_UBI_FASTMAP non-experimental Jesper Nilsson <jesper.nilsson@axis.com> - 2017-03-31 23:30 +0200
            Re: [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental Jesper Nilsson <jesper.nilsson@axis.com> - 2017-04-03 13:20 +0200

#1612082 — [RFC][PATCH] UBI: Make MTD_UBI_FASTMAP non-experimental

MTD_UBI_FASTMAP has been set as experimental since it
was merged back in 2012.

There hasn't been much change in the format,
so we can consider the feature stable and start
being careful about breaking the format.
(This is somewhat of a pre-requisite for anyone actually
using the feature in the real world and depending on it)

Drop the experimental note and the warning text about
the on-flash format not being finalized.

Signed-off-by: Jesper Nilsson <jesper.nilsson@axis.com>
---
 drivers/mtd/ubi/Kconfig | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index f0855ce..019e261 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -57,12 +57,9 @@ config MTD_UBI_BEB_LIMIT
 	  Leave the default value if unsure.
 
 config MTD_UBI_FASTMAP
-	bool "UBI Fastmap (Experimental feature)"
+	bool "UBI Fastmap"
 	default n
 	help
-	   Important: this feature is experimental so far and the on-flash
-	   format for fastmap may change in the next kernel versions
-
 	   Fastmap is a mechanism which allows attaching an UBI device
 	   in nearly constant time. Instead of scanning the whole MTD device it
 	   only has to locate a checkpoint (called fastmap) on the device.
-- 
2.1.4


/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[toc] | [next] | [standalone]

#1612287

Jesper,

Am 29.03.2017 um 17:38 schrieb Jesper Nilsson:
> MTD_UBI_FASTMAP has been set as experimental since it
> was merged back in 2012.
> 
> There hasn't been much change in the format,
> so we can consider the feature stable and start
> being careful about breaking the format.
> (This is somewhat of a pre-requisite for anyone actually
> using the feature in the real world and depending on it)
> 
> Drop the experimental note and the warning text about
> the on-flash format not being finalized.

I fully agree, we can drop this note. But we have to add another
one.
While Fastmap is a nice feature to speed-up the attach time it
comes with a cost. It makes UBI less robust. I saw issues
on NAND chips which misbehaved slightly where UBI was able to
recover when using a full scan but not when Fastmap was used.
The UBI full scan code is paranoid and can sort out problems
very early, with Fastmap enabled you lose this valuable property.

So, users should enable Fastmap only when they absolutely need
a very fast attach time and be very sure that the NAND works as
expected.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]

#1612850

On 03/29/2017 10:04 PM, Richard Weinberger wrote:
> Jesper,
> 
> Am 29.03.2017 um 17:38 schrieb Jesper Nilsson:
>> MTD_UBI_FASTMAP has been set as experimental since it
>> was merged back in 2012.
>>
>> There hasn't been much change in the format,
>> so we can consider the feature stable and start
>> being careful about breaking the format.
>> (This is somewhat of a pre-requisite for anyone actually
>> using the feature in the real world and depending on it)
>>
>> Drop the experimental note and the warning text about
>> the on-flash format not being finalized.
> 
> I fully agree, we can drop this note. But we have to add another
> one.
> While Fastmap is a nice feature to speed-up the attach time it
> comes with a cost. It makes UBI less robust. I saw issues
> on NAND chips which misbehaved slightly where UBI was able to
> recover when using a full scan but not when Fastmap was used.
> The UBI full scan code is paranoid and can sort out problems
> very early, with Fastmap enabled you lose this valuable property.
> 
> So, users should enable Fastmap only when they absolutely need
> a very fast attach time and be very sure that the NAND works as
> expected.

So we should document this with a big fat warning and set fastmap to
default=n ?

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [next] | [standalone]

#1613326

Hi Richard, Marek,

On Thu, Mar 30, 2017 at 12:01:41PM +0200, Marek Vasut wrote:
> On 03/29/2017 10:04 PM, Richard Weinberger wrote:
> > Jesper,
> > 
> > Am 29.03.2017 um 17:38 schrieb Jesper Nilsson:
> >> MTD_UBI_FASTMAP has been set as experimental since it
> >> was merged back in 2012.
> >>
> >> There hasn't been much change in the format,
> >> so we can consider the feature stable and start
> >> being careful about breaking the format.
> >> (This is somewhat of a pre-requisite for anyone actually
> >> using the feature in the real world and depending on it)
> >>
> >> Drop the experimental note and the warning text about
> >> the on-flash format not being finalized.
> > 
> > I fully agree, we can drop this note. But we have to add another
> > one.
> > While Fastmap is a nice feature to speed-up the attach time it
> > comes with a cost. It makes UBI less robust. I saw issues
> > on NAND chips which misbehaved slightly where UBI was able to
> > recover when using a full scan but not when Fastmap was used.
> > The UBI full scan code is paranoid and can sort out problems
> > very early, with Fastmap enabled you lose this valuable property.
> > 
> > So, users should enable Fastmap only when they absolutely need
> > a very fast attach time and be very sure that the NAND works as
> > expected.
> 
> So we should document this with a big fat warning and set fastmap to
> default=n ?

Does this sound reasonable?

Note that this feature makes UBI less robust, since Fastmap does not scan
the full flash, which might lead to problems on misbehaving NAND chips.
Only enable this if the speedup in attach is really important and
you can be sure that the NAND works as expected.


> Best regards,
> Marek Vasut

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[toc] | [prev] | [next] | [standalone]

#1613485

Jesper,

Am 30.03.2017 um 19:39 schrieb Jesper Nilsson:
>> So we should document this with a big fat warning and set fastmap to
>> default=n ?
> 
> Does this sound reasonable?
> 
> Note that this feature makes UBI less robust, since Fastmap does not scan
> the full flash, which might lead to problems on misbehaving NAND chips.
> Only enable this if the speedup in attach is really important and

I'm not a native English speaker, but shouldn't this be
"...if speedup of the attach time is important ..."

> you can be sure that the NAND works as expected.

Looks fine!

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]

#1614294 — [PATCH v2] UBI: Make MTD_UBI_FASTMAP non-experimental

MTD_UBI_FASTMAP has been set as experimental since it
was merged back in 2012.

There hasn't been much change in the format,
so we can consider the feature stable and start
being careful about breaking the format.
(This is somewhat of a pre-requisite for anyone actually
using the feature in the real world and depending on it)

Drop the experimental note and the warning text about
the on-flash format not being finalized, but add a brief
warning that Fastmap actually makes UBI less robust.

Signed-off-by: Jesper Nilsson <jesper.nilsson@axis.com>
---
Changes in v2:
- Add warning that Fastmap making UBI less robust

 drivers/mtd/ubi/Kconfig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
index f0855ce08ed9..8912943b7142 100644
--- a/drivers/mtd/ubi/Kconfig
+++ b/drivers/mtd/ubi/Kconfig
@@ -57,12 +57,9 @@ config MTD_UBI_BEB_LIMIT
 	  Leave the default value if unsure.
 
 config MTD_UBI_FASTMAP
-	bool "UBI Fastmap (Experimental feature)"
+	bool "UBI Fastmap"
 	default n
 	help
-	   Important: this feature is experimental so far and the on-flash
-	   format for fastmap may change in the next kernel versions
-
 	   Fastmap is a mechanism which allows attaching an UBI device
 	   in nearly constant time. Instead of scanning the whole MTD device it
 	   only has to locate a checkpoint (called fastmap) on the device.
@@ -74,6 +71,10 @@ config MTD_UBI_FASTMAP
 	   images are still usable with UBI implementations without
 	   fastmap support. On typical flash devices the whole fastmap fits
 	   into one PEB. UBI will reserve PEBs to hold two fastmaps.
+	   Note that this feature makes UBI less robust, since Fastmap does not scan
+	   the full flash, which might lead to problems on misbehaving NAND chips.
+	   Only enable this if speedup of the attach time is really important
+	   and you can be sure that the NAND works as expected.
 
 	   If in doubt, say "N".
 
-- 
2.1.4


/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[toc] | [prev] | [next] | [standalone]

#1615110

Hi Richard,

On Thu, Mar 30, 2017 at 11:29:15PM +0200, Richard Weinberger wrote:
> Jesper,
> 
> Am 30.03.2017 um 19:39 schrieb Jesper Nilsson:
> >> So we should document this with a big fat warning and set fastmap to
> >> default=n ?
> > 
> > Does this sound reasonable?
> > 
> > Note that this feature makes UBI less robust, since Fastmap does not scan
> > the full flash, which might lead to problems on misbehaving NAND chips.
> > Only enable this if the speedup in attach is really important and
> 
> I'm not a native English speaker, but shouldn't this be
> "...if speedup of the attach time is important ..."
> 
> > you can be sure that the NAND works as expected.
> 
> Looks fine!

As you saw I resent the patch with this formulation added.

However, after thinking about it (and with input from some coworkers),
could we pinpoint the failure case a bit more here?

What is the exact problem behaviour on NAND chips that we're
worried about, and in which case will UBI be less robust if
we don't scan the full flash?

My first reaction was that this was a natural conclusion,
but if the NAND flash is failing, we should either be in the
case that the FASTMAP is corrupted or that the original data
is corrupted. Both should be found by current implementation.
Or am I missing additional failure cases here?

I getting a bit worried about using the feature at all,
even if it seems to work right now...

> Thanks,
> //richard

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web