Re: Race to power off harming SATA SSDs

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Mon, 8 May 2017 13:06:17 +0200
Richard Weinberger <richard.weinberger@gmail.com> wrote:

> On Mon, May 8, 2017 at 12:49 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > Aha, nice, so it looks like ubifs is a step back here.
> >
> > 'clean marker' is a good idea... empty pages have plenty of space.  
> 
> If UBI (not UBIFS) faces an empty block, it also re-erases it.

Unfortunately, that's not the case, though UBI can easily be patched
to do that (see below).

> The EC header is uses as clean marker.

That is true. If the EC header has been written to a block, that means
this block has been correctly erased.

> 
> > How do you handle the issue during regular write? Always ignore last
> > successfully written block?  

I guess UBIFS can know what was written last, because of the log-based
approach + the seqnum stored along with FS nodes, but I'm pretty sure
UBIFS does not re-write the last written block in case of an unclean
mount. Richard, am I wrong?

> 
> The last page of a block is inspected and allowed to be corrupted.

Actually, it's not really about corrupted pages, it's about pages that
might become unreadable after a few reads.

> 
> > Do you handle "paired pages" problem on MLC?  
> 
> Nope, no MLC support in mainline so far.

Richard and I have put a lot of effort to reliably support MLC NANDs in
mainline, unfortunately this projects has been paused. You can access
the last version of our work here [1] if you're interested (it's
clearly not in a shippable state ;-)).

[1]https://github.com/bbrezillon/linux-sunxi/commits/bb/4.7/ubi-mlc

--->8---
diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 93ceea4f27d5..3d76941c9570 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1121,21 +1121,20 @@ static int scan_peb(struct ubi_device *ubi, struct ubi_attach_info *ai,
                        return err;
                goto adjust_mean_ec;
        case UBI_IO_FF_BITFLIPS:
+       case UBI_IO_FF:
+               /*
+                * Always erase the block if the EC header is empty, even if
+                * no bitflips were reported because otherwise we might
+                * expose ourselves to the 'unstable bits' issue described
+                * here:
+                *
+                * http://www.linux-mtd.infradead.org/doc/ubifs.html#L_unstable_bits
+                */
                err = add_to_list(ai, pnum, UBI_UNKNOWN, UBI_UNKNOWN,
                                  ec, 1, &ai->erase);
                if (err)
                        return err;
                goto adjust_mean_ec;
-       case UBI_IO_FF:
-               if (ec_err || bitflips)
-                       err = add_to_list(ai, pnum, UBI_UNKNOWN,
-                                         UBI_UNKNOWN, ec, 1, &ai->erase);
-               else
-                       err = add_to_list(ai, pnum, UBI_UNKNOWN,
-                                         UBI_UNKNOWN, ec, 0, &ai->free);
-               if (err)
-                       return err;
-               goto adjust_mean_ec;
        default:
                ubi_err(ubi, "'ubi_io_read_vid_hdr()' returned unknown code %d",
                        err);