ubi: Reject MLC NAND
diff mbox series

Message ID 20180303104554.5958-1-richard@nod.at
State New
Headers show
Series
  • ubi: Reject MLC NAND
Related show

Commit Message

Richard Weinberger March 3, 2018, 10:45 a.m. UTC
While UBI and UBIFS seem to work at first sight with MLC NAND, you will
most likely lose all your data upon a power-cut or due to read/write
disturb.
In order to protect users from bad surprises, refuse to attach to MLC
NAND.

Cc: stable@vger.kernel.org
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/ubi/build.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Boris Brezillon March 4, 2018, 7:46 p.m. UTC | #1
On Sat,  3 Mar 2018 11:45:54 +0100
Richard Weinberger <richard@nod.at> wrote:

> While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> most likely lose all your data upon a power-cut or due to read/write
> disturb.
> In order to protect users from bad surprises, refuse to attach to MLC
> NAND.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Richard Weinberger <richard@nod.at>

Acked-by: Boris Brezillon <boris.brezillon@bootlin.com>

> ---
>  drivers/mtd/ubi/build.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index e941395de3ae..753494e042d5 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -854,6 +854,17 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Both UBI and UBIFS have been designed for SLC NAND and NOR flashes.
> +	 * MLC NAND is different and needs special care, otherwise UBI or UBIFS
> +	 * will die soon and you will lose all your data.
> +	 */
> +	if (mtd->type == MTD_MLCNANDFLASH) {
> +		pr_err("ubi: refuse attaching mtd%d - MLC NAND is not supported\n",
> +			mtd->index);
> +		return -EINVAL;
> +	}
> +
>  	if (ubi_num == UBI_DEV_NUM_AUTO) {
>  		/* Search for an empty slot in the @ubi_devices array */
>  		for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++)
Pavel Machek March 6, 2018, 11:18 p.m. UTC | #2
On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:
> While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> most likely lose all your data upon a power-cut or due to read/write
> disturb.
> In order to protect users from bad surprises, refuse to attach to MLC
> NAND.
> 
> Cc: stable@vger.kernel.org

That sounds like _really_ bad idea for stable. All it does is it
removes support for hardware that somehow works.

Now... Yes, handling flash is hard, MLC NAND is worst of the
bunch. But...

Read disturb is not unique to MLC, right? Happens to all the flashes,
its just that MLC has tighter margins for error.

Power-cut. UBIFS is just not power-cut safe, right? My notes say that
power-cut during erase could result in flash that contains all 1s, but
will start showing errors very quickly. Again, not MLC specific. Can
be solved with battery...

(And yes, there are some problems specific to MLC, where parts of page
need to be written in specific order. Not sure how it affects
ubifs. But it was not listed as a reason for the patch.)

> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/mtd/ubi/build.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index e941395de3ae..753494e042d5 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -854,6 +854,17 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Both UBI and UBIFS have been designed for SLC NAND and NOR flashes.
> +	 * MLC NAND is different and needs special care, otherwise UBI or UBIFS
> +	 * will die soon and you will lose all your data.
> +	 */
> +	if (mtd->type == MTD_MLCNANDFLASH) {
> +		pr_err("ubi: refuse attaching mtd%d - MLC NAND is not supported\n",
> +			mtd->index);
> +		return -EINVAL;
> +	}
> +
>  	if (ubi_num == UBI_DEV_NUM_AUTO) {
>  		/* Search for an empty slot in the @ubi_devices array */
>  		for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++)
David Oberhollenzer March 7, 2018, 1:57 a.m. UTC | #3
On 03/07/2018 12:18 AM, Pavel Machek wrote:
> Now... Yes, handling flash is hard, MLC NAND is worst of the
> bunch. But...
> 
MLC is not just more flimsy than SLC NAND.

The real problem is that on MLC NAND, pages come in pairs.

Multiple voltage levels inside a single, physical memory cell are used to
encode more than one bit. Instead of just having pages that are twice as big,
the flash exposes them as *two different pages*. Those pages are usually not
ordered sequentially either, but according to a vendor/device specific
pairing scheme.


> Read disturb is not unique to MLC, right? Happens to all the flashes,
> its just that MLC has tighter margins for error.
> 
You don't just have more read disturb on a single page. Reading one page
will also affect the paired page as well.[0]

In addition to that, you also have program disturb. Even *writing successfully*
to the upper page of a pair also causes bit flips on the lower one, affecting
completely different data.[0]

This could be addressed using the experimental ubihealthd that crawls the flash
from time to time (at UBI level), making UBI notice and fix bit flips.[0]


> Power-cut. UBIFS is just not power-cut safe, right? My notes say that
> power-cut during erase could result in flash that contains all 1s, but
> will start showing errors very quickly. Again, not MLC specific. Can
> be solved with battery...
> 
> (And yes, there are some problems specific to MLC, where parts of page
> need to be written in specific order. Not sure how it affects
> ubifs. But it was not listed as a reason for the patch.)
> 

Both UBI and UBIFS were designed to be power cut tolerant.[1]

On MLC however, if you interrupt a write access to the upper page of a pair,
you will also corrupt the lower one, i.e. *data that was already written*,
which is a serious issue.[0]


Richard and Boris did spend a lot of time working on a solution for that at
UBI level which would currently still require a lot of testing and fine
tuning, but sadly we ran out of budget.

The proposed patch tries to prevent further bad surprises for people who try
to use UBI and UBIFS on top of MLC by making sure that UBI _refuses_ to run
on MLC (at least for now).


Regards,

David

[0] http://linux-mtd.infradead.org/doc/ubifs.html#L_ubifs_mlc
[1] http://linux-mtd.infradead.org/doc/ubifs.html#L_powercut
Richard Weinberger March 7, 2018, 8:01 a.m. UTC | #4
Pavel,

Am Mittwoch, 7. März 2018, 00:18:05 CET schrieb Pavel Machek:
> On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:
> > While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> > most likely lose all your data upon a power-cut or due to read/write
> > disturb.
> > In order to protect users from bad surprises, refuse to attach to MLC
> > NAND.
> > 
> > Cc: stable@vger.kernel.org
> 
> That sounds like _really_ bad idea for stable. All it does is it
> removes support for hardware that somehow works.

MLC is not supported and does not work. Full stop.
If someone manages to get it somehow work, either with hardware or software 
hacks they are on their own.
Having it in stable is the only chance we have to get it into vendor kernels.

All this patch does is representing the current state of UBI/UBIFS.

In the last months I got a way too much boards with MLC NAND on my desk where 
UBIFS broke. Customers wished they had known before...

Thanks,
//richard
Artem Bityutskiy March 7, 2018, 3:24 p.m. UTC | #5
On Wed, 2018-03-07 at 09:01 +0100, Richard Weinberger wrote:
> Pavel,
> 
> Am Mittwoch, 7. März 2018, 00:18:05 CET schrieb Pavel Machek:
> > On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:
> > > While UBI and UBIFS seem to work at first sight with MLC NAND,
> > > you will
> > > most likely lose all your data upon a power-cut or due to
> > > read/write
> > > disturb.
> > > In order to protect users from bad surprises, refuse to attach to
> > > MLC
> > > NAND.
> > > 
> > > Cc: stable@vger.kernel.org
> > 
> > That sounds like _really_ bad idea for stable. All it does is it
> > removes support for hardware that somehow works.
> 
> MLC is not supported and does not work. Full stop.

Ack.
Pavel Machek March 7, 2018, 9:43 p.m. UTC | #6
On Wed 2018-03-07 09:01:16, Richard Weinberger wrote:
> Pavel,
> 
> Am Mittwoch, 7. März 2018, 00:18:05 CET schrieb Pavel Machek:
> > On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:
> > > While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> > > most likely lose all your data upon a power-cut or due to read/write
> > > disturb.
> > > In order to protect users from bad surprises, refuse to attach to MLC
> > > NAND.
> > > 
> > > Cc: stable@vger.kernel.org
> > 
> > That sounds like _really_ bad idea for stable. All it does is it
> > removes support for hardware that somehow works.
> 
> MLC is not supported and does not work. Full stop.
> If someone manages to get it somehow work, either with hardware or software 
> hacks they are on their own.
> Having it in stable is the only chance we have to get it into vendor
> kernels.

Can you show how it meets the stable kernel criteria? They are
documented in tree. This should not be in stable.

And I'd like to see changelog improved. Real reason MLC is not
supported is upper/lower page parts on MLC. And real fix to work with
bigger pages in UBI.


									Pavel
Steve deRosier March 7, 2018, 10:08 p.m. UTC | #7
Hi Pavel,

On Wed, Mar 7, 2018 at 1:43 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2018-03-07 09:01:16, Richard Weinberger wrote:
>> Pavel,
>>
>> Am Mittwoch, 7. März 2018, 00:18:05 CET schrieb Pavel Machek:
>> > On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:
>> > > While UBI and UBIFS seem to work at first sight with MLC NAND, you will
>> > > most likely lose all your data upon a power-cut or due to read/write
>> > > disturb.
>> > > In order to protect users from bad surprises, refuse to attach to MLC
>> > > NAND.
>> > >
>> > > Cc: stable@vger.kernel.org
>> >
>> > That sounds like _really_ bad idea for stable. All it does is it
>> > removes support for hardware that somehow works.
>>
>> MLC is not supported and does not work. Full stop.
>> If someone manages to get it somehow work, either with hardware or software
>> hacks they are on their own.
>> Having it in stable is the only chance we have to get it into vendor
>> kernels.
>
> Can you show how it meets the stable kernel criteria? They are
> documented in tree. This should not be in stable.
>
> And I'd like to see changelog improved. Real reason MLC is not
> supported is upper/lower page parts on MLC. And real fix to work with
> bigger pages in UBI.
>

To clarify one thing: the reason for this is MLC has actually never
been supported, nor worked properly. The fact that it kinda worked was
incidental and the cause of major problems for people due to that not
being clear. This patch only makes it explicit and avoids people
mistakenly trying to use UBIFS on MLC flash and risking their data and
products. To me, that's what's important.

This is an important patch, even if all it does is keep people from
loosing data. It also changes the conversation from "I have a
corrupted UBIFS device, BTW it's on MLC..." to "What can we do to get
UBIFS to work on MLC".

I don't know what the stable criteria is with re: to this patch. But
what I do know is if it doesn't go back into the various stables,
there will be manufacturers who will continue to try to use UBIFS on
MLC in ignorance for the next several years until the current stable
kernels EOL, despite there being a known patch that would make it
immediately obvious they shouldn't.

Thanks,
- Steve
David Woodhouse March 7, 2018, 10:11 p.m. UTC | #8
On Wed, 2018-03-07 at 14:08 -0800, Steve deRosier wrote:
> 
> To clarify one thing: the reason for this is MLC has actually never
> been supported, nor worked properly. The fact that it kinda worked was
> incidental and the cause of major problems for people due to that not
> being clear. This patch only makes it explicit and avoids people
> mistakenly trying to use UBIFS on MLC flash and risking their data and
> products. To me, that's what's important.
> 
> This is an important patch, even if all it does is keep people from
> loosing data. It also changes the conversation from "I have a
> corrupted UBIFS device, BTW it's on MLC..." to "What can we do to get
> UBIFS to work on MLC".

This is a bug fix.

UBI on MLC never worked. It was a bug that we ever permitted it. This
is now fixed.

Actually making UBI on MLC *work* would be a feature, and wouldn't be
backported to stable. But this one is *absolutely* appropriate for a
stable backport.
Pavel Machek March 7, 2018, 10:17 p.m. UTC | #9
On Wed 2018-03-07 22:11:13, David Woodhouse wrote:
> 
> 
> On Wed, 2018-03-07 at 14:08 -0800, Steve deRosier wrote:
> > 
> > To clarify one thing: the reason for this is MLC has actually never
> > been supported, nor worked properly. The fact that it kinda worked was
> > incidental and the cause of major problems for people due to that not
> > being clear. This patch only makes it explicit and avoids people
> > mistakenly trying to use UBIFS on MLC flash and risking their data and
> > products. To me, that's what's important.
> > 
> > This is an important patch, even if all it does is keep people from
> > loosing data. It also changes the conversation from "I have a
> > corrupted UBIFS device, BTW it's on MLC..." to "What can we do to get
> > UBIFS to work on MLC".

Well, for -stable I'd suggest printk(KERN_ALERT ...) but keep the
system running.

> This is a bug fix.
> 
> UBI on MLC never worked. It was a bug that we ever permitted it. This
> is now fixed.

Yeah, well, so lets say I have a working hardware (maybe using
read-only UBI on MLC), update to next stable kernel, and now kernel
refuses to see the partition.

I'll certainly not consider this patch a bug fix.

Removing support for hardware that "only works by mistake" may be good
idea, but maybe it is slightly too surprising for a -stable.

									Pavel
Boris Brezillon March 7, 2018, 10:30 p.m. UTC | #10
Hi Pavel,

On Wed, 7 Mar 2018 23:17:33 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> On Wed 2018-03-07 22:11:13, David Woodhouse wrote:
> > 
> > 
> > On Wed, 2018-03-07 at 14:08 -0800, Steve deRosier wrote:  
> > > 
> > > To clarify one thing: the reason for this is MLC has actually never
> > > been supported, nor worked properly. The fact that it kinda worked was
> > > incidental and the cause of major problems for people due to that not
> > > being clear. This patch only makes it explicit and avoids people
> > > mistakenly trying to use UBIFS on MLC flash and risking their data and
> > > products. To me, that's what's important.
> > > 
> > > This is an important patch, even if all it does is keep people from
> > > loosing data. It also changes the conversation from "I have a
> > > corrupted UBIFS device, BTW it's on MLC..." to "What can we do to get
> > > UBIFS to work on MLC".  
> 
> Well, for -stable I'd suggest printk(KERN_ALERT ...) but keep the
> system running.
> 
> > This is a bug fix.
> > 
> > UBI on MLC never worked. It was a bug that we ever permitted it. This
> > is now fixed.  
> 
> Yeah, well, so lets say I have a working hardware (maybe using
> read-only UBI on MLC), update to next stable kernel, and now kernel
> refuses to see the partition.

Read-only does not save you from the read-disturb issue, and you even
have to take care of programming the full erase-block on some MLC
NANDs, which AFAIR is not done when updating a static volume.

I have one simple question: did you ever play with MLC NANDs or are you
just trolling? If you had, like Richard and I did when working on MLC
support, I'm pretty sure you wouldn't play this "don't backport to
stable" card.

Now, if you volunteer to add reliable MLC support, I can send you a
few boards to play with. I even have a "working but not so tested PoC"
here [1] if you want to finish the job, but please don't do the mistake
of thinking the fix is that simple.

> 
> I'll certainly not consider this patch a bug fix.

And apparently a lot of people disagree with you on this point, and I
guess all of them had problems with MLC NANDs.

> 
> Removing support for hardware that "only works by mistake" may be good
> idea, but maybe it is slightly too surprising for a -stable.

I wouldn't say "work by mistake" but "seems to work at first but in the
end breaks", so definitely a candidate for -stable IMO.

Regards,

Boris

[1]https://github.com/bbrezillon/linux/tree/nand/mlc
Boris Brezillon March 7, 2018, 10:40 p.m. UTC | #11
On Wed, 7 Mar 2018 22:43:42 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> On Wed 2018-03-07 09:01:16, Richard Weinberger wrote:
> > Pavel,
> > 
> > Am Mittwoch, 7. März 2018, 00:18:05 CET schrieb Pavel Machek:  
> > > On Sat 2018-03-03 11:45:54, Richard Weinberger wrote:  
> > > > While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> > > > most likely lose all your data upon a power-cut or due to read/write
> > > > disturb.
> > > > In order to protect users from bad surprises, refuse to attach to MLC
> > > > NAND.
> > > > 
> > > > Cc: stable@vger.kernel.org  
> > > 
> > > That sounds like _really_ bad idea for stable. All it does is it
> > > removes support for hardware that somehow works.  
> > 
> > MLC is not supported and does not work. Full stop.
> > If someone manages to get it somehow work, either with hardware or software 
> > hacks they are on their own.
> > Having it in stable is the only chance we have to get it into vendor
> > kernels.  
> 
> Can you show how it meets the stable kernel criteria? They are
> documented in tree. This should not be in stable.
> 
> And I'd like to see changelog improved. Real reason MLC is not
> supported is upper/lower page parts on MLC. And real fix to work with
> bigger pages in UBI.

Come on! Don't you think this would have been fixed already if it was
that easy?! Have you looked at an MLC datasheet to see how paired pages
are combined? If you had you would now that paired pages are almost all
the time not contiguous, thus preventing the trick you're suggesting
here. Please document yourself before doing such presumptuous
statements (you can have a look at these slides if you want some details
about why this is not so simple [1]).

I'm definitely not saying supporting MLC NANDs in Linux is impossible,
and if you're interested in working on this topic I'd be happy to help.
But please don't block this patch without understanding what supporting
MLC NANDs implies.

[1]https://events.static.linuxfound.org/sites/events/files/slides/ubi-mlc.pdf
Willy Tarreau March 7, 2018, 11:27 p.m. UTC | #12
Hi Boris,

On Wed, Mar 07, 2018 at 11:30:57PM +0100, Boris Brezillon wrote:
> I have one simple question: did you ever play with MLC NANDs or are you
> just trolling? If you had, like Richard and I did when working on MLC
> support, I'm pretty sure you wouldn't play this "don't backport to
> stable" card.

Just adding my two cents here, I've always had stability issues with
ubifs on NAND and I never knew what type of NAND I've had in these
devices (eg: Iomega Iconnect with 256 MB NAND IIRC), so maybe this
has never been relevant, maybe it has been, I don't know. However,
as a stable maintainer I also know that we want to encourage people
to always keep their kernels up to date with latest fixes, and that
if there is the slightest risk that a loss of functionality makes
people revert and stick to an older, working version, keeping their
bugs forever, it's twice as worse, because :
  - they still run on the bug you wanted to fix
  - they are now exposed to all bugs fixed later.

And we all do this as users, thinking "I'll bisect and report tomorrow"
and priorities change, let's be honnest. Thus I think that if you are
absolutely certain that it's impossible that people are accidently using
this combination, it's probably fine, but if people are using it, you're
just displacing the burden on the stable team who will have to explain
to each and every user complaining "my system stopped booting after an
upgrade to 4.x.y". A big red alert polluting the logs and console every
minute, and a pause at boot saying "your NAND, your data and all your
kids photos will die soon if you don't switch to another FS" is more
productive as users will be less tempted to blindly revert and will at
least ask what the problem is and what their solutions are.

There's nothing more frustrating than a regression in a stable branch,
even for something that was not supposed to work but did by accident.

> I wouldn't say "work by mistake" but "seems to work at first but in the
> end breaks", so definitely a candidate for -stable IMO.

Well, removing support definitely makes the end closer and possibly even
prevents the user from recovering their data. I know that data loss is
terrible, but data confiscation is similar from the user's point of view.

Users don't know the technical details so they will do all things we
often consider stupid or impossible, but when warned they know that the
risk is on their side and they cannot put the fault at anybody anymore.
It tends to work better.

Just my two cents,
Willy
Linus Walleij March 8, 2018, 1:42 p.m. UTC | #13
On Sat, Mar 3, 2018 at 11:45 AM, Richard Weinberger <richard@nod.at> wrote:

> While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> most likely lose all your data upon a power-cut or due to read/write
> disturb.
>
> In order to protect users from bad surprises, refuse to attach to MLC
> NAND.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Richard Weinberger <richard@nod.at>

I'm sorry to disturb in this interesting discussion about what
"stable" really means as in "stable kernel".  Stable for who and
in what sense, that seems to be the question.

But my main problem here is to understand who the consumers
of the MLC NAND devices really are.

I hear some talk here about lab boards. But where is this
really deployed, large-scale? And who are the people that
will have their devices potentially not booting after this patch?

I am pretty sure these people are board support or
customization consultants with work being done for some
certain products, and not hobbyists and even less end
consumers, right?

What kind of devices are MLC NANDs being deployed in?
Certainly not laptops, tablets and phones, they all use eMMC
and even start to venture into UFS (unified flash storage).

What is using these flashes? Routers and switches? NAS boxes?
Industrial control? Automotive?

Or are (God forbid, but would not surprise me) talking about a
Linux instance running inside of eMMCs or UFS devices?

Yours,
Linus Walleij
Richard Weinberger March 8, 2018, 2:43 p.m. UTC | #14
Linus,

Am Donnerstag, 8. März 2018, 14:42:16 CET schrieb Linus Walleij:
> On Sat, Mar 3, 2018 at 11:45 AM, Richard Weinberger <richard@nod.at> wrote:
> > While UBI and UBIFS seem to work at first sight with MLC NAND, you will
> > most likely lose all your data upon a power-cut or due to read/write
> > disturb.
> > 
> > In order to protect users from bad surprises, refuse to attach to MLC
> > NAND.
> > 
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Richard Weinberger <richard@nod.at>
> 
> I'm sorry to disturb in this interesting discussion about what
> "stable" really means as in "stable kernel".  Stable for who and
> in what sense, that seems to be the question.
> 
> But my main problem here is to understand who the consumers
> of the MLC NAND devices really are.
> 
> I hear some talk here about lab boards. But where is this
> really deployed, large-scale? And who are the people that
> will have their devices potentially not booting after this patch?
> 
> I am pretty sure these people are board support or
> customization consultants with work being done for some
> certain products, and not hobbyists and even less end
> consumers, right?

Correct.
I saw vendor specific kernels that have hardware and software hacks to make 
UBI on MLC NAND somehow work.
Somehow in terms of, the filesystem will die just a little later.

But these people do _not_ run a vanilla (stable) kernel.
We support mainline, nothing more, nothing less.

> What kind of devices are MLC NANDs being deployed in?
> Certainly not laptops, tablets and phones, they all use eMMC
> and even start to venture into UFS (unified flash storage).
> 
> What is using these flashes? Routers and switches? NAS boxes?
> Industrial control? Automotive?
> 
> Or are (God forbid, but would not surprise me) talking about a
> Linux instance running inside of eMMCs or UFS devices?

We need to clarify that even more. Upstream UBI and UBIFS cannot work with MLC 
NAND correctly. Any such product would die within days...
I have many MLC boards on my desk, by running a mainline kernel on them, I can 
kill every single one within a few minutes, either by plugging the power or 
triggering read-disturb.

As stated by David Woodhouse, it was a huge mistake by UBI to not reject MLC 
NAND from the very beginning.
The sole purpose of this patch is fixing that mistake and make it very clear.

Thanks,
//richard
Artem Bityutskiy March 8, 2018, 3:01 p.m. UTC | #15
On Thu, 2018-03-08 at 15:43 +0100, Richard Weinberger wrote:
> As stated by David Woodhouse, it was a huge mistake by UBI to not
> reject MLC 
> NAND from the very beginning.

Correction: when we were developing UBI/UBIFS and upstreamed them, MLC
was widely used yet we did not really know about it. So there was
nothing to reject yet.

The mistake is that we did not add the reject timely. When people
started reporting MLC issues we were answering that UBI/UBIFS stack
needs more work to make MLC safe to use, and we hoped someone would do
the work.

Artem.
Richard Weinberger March 8, 2018, 3:28 p.m. UTC | #16
Artem,

Am Donnerstag, 8. März 2018, 16:01:15 CET schrieb Artem Bityutskiy:
> On Thu, 2018-03-08 at 15:43 +0100, Richard Weinberger wrote:
> > As stated by David Woodhouse, it was a huge mistake by UBI to not
> > reject MLC
> > NAND from the very beginning.
> 
> Correction: when we were developing UBI/UBIFS and upstreamed them, MLC
> was widely used yet we did not really know about it. So there was
> nothing to reject yet.

You mean *not* widely used?
 
> The mistake is that we did not add the reject timely. When people
> started reporting MLC issues we were answering that UBI/UBIFS stack
> needs more work to make MLC safe to use, and we hoped someone would do
> the work.

True.
TBH Boris and I also thought that adding MLC support is not a that big deal...

Thanks,
//richard
Artem Bityutskiy March 8, 2018, 3:34 p.m. UTC | #17
On Thu, 2018-03-08 at 16:28 +0100, Richard Weinberger wrote:
> You mean *not* widely used?

Yes, indeed, thanks for correction. MLC just wasn't there yet when
UBI/UBIFS were upstreamed.

Patch
diff mbox series

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index e941395de3ae..753494e042d5 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -854,6 +854,17 @@  int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
 		return -EINVAL;
 	}
 
+	/*
+	 * Both UBI and UBIFS have been designed for SLC NAND and NOR flashes.
+	 * MLC NAND is different and needs special care, otherwise UBI or UBIFS
+	 * will die soon and you will lose all your data.
+	 */
+	if (mtd->type == MTD_MLCNANDFLASH) {
+		pr_err("ubi: refuse attaching mtd%d - MLC NAND is not supported\n",
+			mtd->index);
+		return -EINVAL;
+	}
+
 	if (ubi_num == UBI_DEV_NUM_AUTO) {
 		/* Search for an empty slot in the @ubi_devices array */
 		for (ubi_num = 0; ubi_num < UBI_MAX_DEVICES; ubi_num++)