[RFC PATCH] openbmc: Use JFFS2 for RW overlay filesystem

Milton Miller II miltonm at us.ibm.com
Tue Feb 16 11:10:56 AEDT 2016


Hi Andrew.

You got ahead of me and explored some issues, thanks.  I wasn't thinking
about some of the details but I had plans for some others, and had been
manually changing the type on my own system (causing someone to re-flash
when it didn't boot for them.  cie la vie.


On  02/15/2016 around 01:54AM in some timezone,  Andrew Jeffery wrote:
> Subject: [RFC PATCH] openbmc: Use JFFS2 for RW overlay filesystem
> 
> We can enable JFFS2 support on mtd6 (the RW mtd partition) with a
> (first-pass) kernel patch[1] applied to work-around a 'stutter' in the
> (optimised) ARM mmiocpy() implementation.

optimised is flagged in my spell checker.

> 
> Whilst the original scripting surrounding the filesystem selection aimed
> to be generic, creating and mounting JFFS2 has some warts that seems to
> require special-case logic.

Or at least some versions of the tools think so :-)

> 
> [1] Unmerged, but tested: https://lists.ozlabs.org/pipermail/openbmc/2016-February/001874.html
> 
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
>  meta-phosphor/classes/image-overlay.bbclass        |  3 ++-
>  meta-phosphor/classes/obmc-phosphor-image.bbclass  |  2 ++
>  .../obmc-phosphor-image_types_uboot.bbclass        | 12 ++++++++++--
>  .../obmc-phosphor-initfs/files/obmc-init.sh        | 22 +++++++++++++++-------
>  .../obmc-phosphor-initfs/files/obmc-update.sh      | 13 +++++++++----
>  5 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/meta-phosphor/classes/image-overlay.bbclass b/meta-phosphor/classes/image-overlay.bbclass
> index 1c6137d..5043e82 100644
> --- a/meta-phosphor/classes/image-overlay.bbclass
> +++ b/meta-phosphor/classes/image-overlay.bbclass
> @@ -3,7 +3,8 @@ INITRD_IMAGE ?= "core-image-minimal-initramfs"
>  INITRD ?= "${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE}-${MACHINE}.cpio.${INITRD_CTYPE}${uboot}"
>  
>  IMAGE_BASETYPE ?= "squashfs-xz"
> -OVERLAY_BASETYPE ?= "ext4"
> +OVERLAY_BASETYPE ?= "jffs2"
> +OVERLAY_BASETYPE_OPTS ?= "--pagesize=4096 --little-endian --squash"
>  

Lots of options on that mkfs.   Not sure what the --squash flag would be for.

>  IMAGE_TYPES_${PN} += "${IMAGE_BASETYPE}"
>  
diff --git a/meta-phosphor/classes/obmc-phosphor-image.bbclass b/meta-phosphor/classes/obmc-phosphor-image.bbclass
index 9a13f7f..8afa1ea 100644
--- a/meta-phosphor/classes/obmc-phosphor-image.bbclass
+++ b/meta-phosphor/classes/obmc-phosphor-image.bbclass
@@ -38,4 +38,6 @@ IMAGE_INSTALL += " \
        packagegroup-obmc-phosphor-apps-extras \
        i2c-tools \
        screen \
+       mtd-utils-jffs2 \
+       mtd-utils-misc \
        "

You say below that there is no fsck.jffs2, so why are you adding 
mtd-utils instead of using the busybox built-in files?  Also, the 
u-boot utilities seem to be ok to compile with out adding these to 
the package list.

> diff --git a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass b/meta-phosphor/classes/obmc-phosphor-
> image_types_uboot.bbclass
> index c390c36..dc69dc1 100644
> --- a/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> +++ b/meta-phosphor/classes/obmc-phosphor-image_types_uboot.bbclass
> @@ -51,8 +51,16 @@ do_generate_flash() {
>         fi
>  
>         oe_mkimage  "${initrd}" "${INITRD_CTYPE}" || bbfatal "oe_mkimage initrd"
> -       dd if=/dev/zero of=${ddir}/${rwfs} bs=1k count=${RWFS_SIZE}
> -       mkfs.${OVERLAY_BASETYPE} -b 4096 -F -O^huge_file ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
> +
> +       if [ x"${OVERLAY_BASETYPE}" = xjffs2 ]; then
> +              mkfs.${OVERLAY_BASETYPE} ${OVERLAY_BASETYPE_OPTS} \
> +                     --pad=$(expr ${RWFS_SIZE} \* 1024) \
> +                     -o ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
> +       else
> +              dd if=/dev/zero of=${ddir}/${rwfs} bs=1k count=${RWFS_SIZE}
> +              mkfs.${OVERLAY_BASETYPE} ${OVERLAY_BASETYPE_OPTS} \
> +                     ${ddir}/${rwfs} || bbfatal "mkfs rwfs"
> +       fi
> 

That seems like a lot of logic including special caseing the type but then using the variable, 
and some kind of --pad option.   However, it was pointed out to me while I originally used 
the -j option on the busybox flash_eraseall  command, which writes 1985 to each erase block, 
the Facebook and other code just did erase_all and mount.  Investigation shows jffs2 seems 
to be very happy with a blank NOR flash; I suspect the markers may be more for NAND and/or 
RAM.

Also while there were some ext4 specific options not in variables they should be moved there
before switching the type so it will be easier to go back (if, for instance, someone wants 
to use managed nand eg emmc or usb for their read-write storage).

So, my suggestion here for empty mkfs.jffs2:

dd (size options) | tr '\x00' '\xff'


> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh b/meta-phosphor/common
> /recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> index 6750de3..226b199 100644
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-init.sh
> @@ -65,18 +65,26 @@ then
>  	cp /run/mtd:u-boot-env /run/fw_env
>  fi
>  
> +rofst=squashfs
>  rofs=$(findmtd rofs)
> -rwfs=$(findmtd rwfs)
> -
> +roopts=ro
>  rodev=/dev/mtdblock${rofs#mtd}
> -rwdev=/dev/mtdblock${rwfs#mtd}
>  
> -rofst=squashfs
> -rwfst=ext4
> -roopts=ro
> +rwfst=jffs2
> +rwfs=$(findmtd rwfs)
>  rwopts=rw
> +rwdev=${rwfs}
> +

You seem to have reordered the lines for a different grouping.  That's probably ok but please do it in a seperate patch either before or after changing the file system type.

> +# mount.jffs2 requires the device node basename, not the absolute device node
> +# path. But still support alternative filesystems through /dev/mtdblockX.
> +if [ x"$rwfst" != xjffs2 ]; then
> +	rwdev=/dev/mtdblock${rwdev#mtd}
> +fi

The kernel mount system call is very happy to accept the /dev/mtdblockX and 
I have been doing all my testing by just changing rwfst.  If the mount.jffs2 
that you added from mtdutils does not accept this then not only is it a waste 
of space but actively harmeful.  

Please remove that broken code from the build and try without.  Busybox mount
seems to be fine when the fstype is listed.

Alternatively the kernel accepts mtd:<name> where name is the mtd name.  While 
that is useful I have not checked if squashfs is happy with that, and it makes 
it harder to switch to other fs types (eg loop mount).

 
> init=/sbin/init
> +
> +# There is no fsck for jffs2 - For rwfst=jffs2 just produces a warning in the
> +# output about being unable to find the binary and the boot continues.

That is true, or we could create a link to /bin/true.  The comment it not 
given to the screen where the user sees the message.

 
>  Mounting read-write $rwdev filesystem failed.  Please fix and run
> -	mount $rwdev $rwdir -t $rwfs -o $rwopts
> +	mount $rwdev $rwdir -t $rwfst -o $rwopts
>  to to continue, or do change nothing to run from RAM for this boot.
>  HERE

Good catch, I must not have tried to cut and paste that.
It should arguably be a seperate patch.


> diff --git a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> index face06d..2bba4b0 100755
> --- a/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> +++ b/meta-phosphor/common/recipes-phosphor/obmc-phosphor-initfs/files/obmc-update.sh
> @@ -35,13 +35,18 @@ findmtd() {

[ Same comments about variable order changes and updates in update script ]


But I also had plans to have the concept of "old" and "new" rwfst , where update
would check for the fs mounting as the old type before but be mounted with the
new type after the update.

Its a bit tricky because jffs2 will try to mount anything but complain about erase 
marks on every block that it can not parse.  ext4 relies on magic numbers and so 
should always be tried before jffs2.

Also I was thinking about taking advantage of flash_eraseall being an alias for 
mkfs.jffs2 and doing the wipe in more cases, maybe by an alternate trigger file.  I
don't want to do it for a u-boot-env update but probably should when we do 
image-rofs.


I have a patch series to split the whitelist into pieces that affects mostly 
update (and a rename in init).  I didn't post it because I wanted a chance to 
review it and had yet to investigate making moving the files in the build 
system.  I will probably send that tomorrow.  Actually the merge looks easy its 
all contained to the save loop block; most of the work is in the recipe.



Thanks,
milton



More information about the openbmc mailing list