[v2] mkfs.ubifs: add "-F" option for "free-space fixup"

This adds a superblock flag indicating that "free-space fixup" is needed, and
allows it to be set by the user via the "-F" command-line option.  The first
time the filesystem is mounted, this flag will trigger a one-time re-mapping of
all LEBs containing free space.  This fixes problems seen on some NAND flashes
when a non-UBIFS-aware flash programmer is used.


Changes since v1: renamed "LEB fixup" to "free space fixup"

Signed-off-by: Matthew L. Creech <mlcreech@gmail.com>
---
 mkfs.ubifs/mkfs.ubifs.c  |   11 ++++++++++-
 mkfs.ubifs/ubifs-media.h |    2 ++
 mkfs.ubifs/ubifs.h       |    2 ++
 3 files changed, 14 insertions(+), 1 deletions(-)

Hi Matthew,

I keep meaning to get around to testing the free-space fixup patches
as promised. Sorry.

For now all I can offer is some superficial criticisms of this patch.
Sorry again.

I think your mail client wrapped the patch. When I try to 'git am' it
onto "e15b521 fs-tests: integck: print error number in pcv messages" I
get

Applying: mkfs.ubifs: add "-F" option for "free-space fixup"
fatal: corrupt patch at line 30
Patch failed at 0001 mkfs.ubifs: add "-F" option for "free-space fixup"
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

On Wed, May 4, 2011 at 6:14 PM, Matthew L. Creech <mlcreech@gmail.com> wrote:
> This adds a superblock flag indicating that "free-space fixup" is needed, and
> allows it to be set by the user via the "-F" command-line option.  The first
> time the filesystem is mounted, this flag will trigger a one-time re-mapping of
> all LEBs containing free space.  This fixes problems seen on some NAND flashes
> when a non-UBIFS-aware flash programmer is used.
>
>
> Changes since v1: renamed "LEB fixup" to "free space fixup"

Put stuff like 'changes since' below a tearline ('---') so it doesn't
make it into the commit message

> Signed-off-by: Matthew L. Creech <mlcreech@gmail.com>
> ---
>  mkfs.ubifs/mkfs.ubifs.c  |   11 ++++++++++-
>  mkfs.ubifs/ubifs-media.h |    2 ++
>  mkfs.ubifs/ubifs.h       |    2 ++
>  3 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
> index a306dd6..96e17a3 100644
> --- a/mkfs.ubifs/mkfs.ubifs.c
> +++ b/mkfs.ubifs/mkfs.ubifs.c
> @@ -132,7 +132,7 @@ static struct inum_mapping **hash_table;
>  /* Inode creation sequence number */
>  static unsigned long long creat_sqnum;
>
> -static const char *optstring = "d:r:m:o:D:h?vVe:c:g:f:p:k:x:X:j:R:l:j:UQq";
> +static const char *optstring = "d:r:m:o:D:h?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQq";
>
>  static const struct option longopts[] = {
>        {"root",               1, NULL, 'r'},
> @@ -150,6 +150,7 @@ static const struct option longopts[] = {
>        {"compr",              1, NULL, 'x'},
>        {"favor-percent",      1, NULL, 'X'},
>        {"fanout",             1, NULL, 'f'},
> +       {"space-fixup",        0, NULL, 'F'},
>        {"keyhash",            1, NULL, 'k'},
>        {"log-lebs",           1, NULL, 'l'},
>        {"orph-lebs",          1, NULL, 'p'},
> @@ -183,6 +184,7 @@ static const char *helptext =
>  "                         how many percent better zlib should
> compress to make\n"

here

>  "                         mkfs.ubifs use zlib instead of LZO (default 20%)\n"
>  "-f, --fanout=NUM         fanout NUM (default: 8)\n"
> +"-F, --space-fixup        fixup free space on first mount (needed for
> some NANDs)\n"

here

>  "-k, --keyhash=TYPE       key hash type - \"r5\" or \"test\"
> (default: \"r5\")\n"

and here

are detrimental wraps

>  "-p, --orph-lebs=COUNT    count of erase blocks for orphans (default: 1)\n"
>  "-D, --devtable=FILE      use device table FILE\n"
> @@ -537,6 +539,7 @@ static int get_options(int argc, char**argv)
>        c->max_leb_cnt = -1;
>        c->max_bud_bytes = -1;
>        c->log_lebs = -1;
> +       c->space_fixup = 0;
>
>        while (1) {
>                opt = getopt_long(argc, argv, optstring, longopts, &i);
> @@ -610,6 +613,9 @@ static int get_options(int argc, char**argv)
>                        if (*endp != '\0' || endp == optarg || c->fanout <= 0)
>                                return err_msg("bad fanout %s", optarg);
>                        break;
> +               case 'F':
> +                       c->space_fixup = 1;
> +                       break;
>                case 'l':
>                        c->log_lebs = strtol(optarg, &endp, 0);
>                        if (*endp != '\0' || endp == optarg || c->log_lebs <= 0)
> @@ -758,6 +764,7 @@ static int get_options(int argc, char**argv)
>                                                "r5" : "test");
>                printf("\tfanout:       %d\n", c->fanout);
>                printf("\torph_lebs:    %d\n", c->orph_lebs);
> +               printf("\tspace_fixup:  %d\n", c->space_fixup);
>        }
>
>        if (validate_options())
> @@ -1997,6 +2004,8 @@ static int write_super(void)
>        }
>        if (c->big_lpt)
>                sup.flags |= cpu_to_le32(UBIFS_FLG_BIGLPT);
> +       if (c->space_fixup)
> +               sup.flags |= cpu_to_le32(UBIFS_FLG_SPACE_FIXUP);
>
>        return write_node(&sup, UBIFS_SB_NODE_SZ, UBIFS_SB_LNUM, UBI_LONGTERM);
>  }
> diff --git a/mkfs.ubifs/ubifs-media.h b/mkfs.ubifs/ubifs-media.h
> index a9ecbd9..fe62d0e 100644
> --- a/mkfs.ubifs/ubifs-media.h
> +++ b/mkfs.ubifs/ubifs-media.h
> @@ -373,9 +373,11 @@ enum {
>  * Superblock flags.
>  *
>  * UBIFS_FLG_BIGLPT: if "big" LPT model is used if set
> + * UBIFS_FLG_SPACE_FIXUP: first-mount "fixup" of free space within LEBs needed
>  */
>  enum {
>        UBIFS_FLG_BIGLPT = 0x02,
> +       UBIFS_FLG_SPACE_FIXUP = 0x04,
>  };
>
>  /**
> diff --git a/mkfs.ubifs/ubifs.h b/mkfs.ubifs/ubifs.h
> index 5c29046..f94a52c 100644
> --- a/mkfs.ubifs/ubifs.h
> +++ b/mkfs.ubifs/ubifs.h
> @@ -317,6 +317,7 @@ struct ubifs_znode
>  * @nhead_lnum: LEB number of LPT head
>  * @nhead_offs: offset of LPT head
>  * @big_lpt: flag that LPT is too big to write whole during commit
> + * @space_fixup: flag indicating that free space in LEBs needs to be cleaned up
>  * @lpt_sz: LPT size
>  *
>  * @ltab_lnum: LEB number of LPT's own lprops table
> @@ -394,6 +395,7 @@ struct ubifs_info
>        int nhead_lnum;
>        int nhead_offs;
>        int big_lpt;
> +       int space_fixup;
>        long long lpt_sz;
>
>        int ltab_lnum;
> --
> 1.6.3.3
>
>
> --
> Matthew L. Creech
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>

I am planning to do actual testing of this flag on the da850 at the
end of next week.

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca

On Wed, May 11, 2011 at 4:09 PM, Ben Gardiner
<bengardiner@nanometrics.ca> wrote:
>
> I think your mail client wrapped the patch. When I try to 'git am' it
> onto "e15b521 fs-tests: integck: print error number in pcv messages" I
> get
>

Yes, I'm sorry about that!  Artem pointed out the same problem with my
kernel patches (it's a gmail wrapping issue).  I'll reply with a
proper version.