fs/ext2: cleanups and improvements

PATCH New Yann E. MORIN 2 participants 14 messages May 1, 2017 – May 13, 2017 ext2 file system

lore

Share & tools

lore

Yann E. MORIN

author

2017-05-01 15:58 UTC #0 lore raw

cover fs/ext2: cleanups and improvements

Hello All!

This series brings in some cleanups about our ext2 filesystem handling, and a few enhancements, of which the ability to reserve 0 block for root, and set the block size.

Regards,
Yann E. MORIN.

The following changes since commit 5f14d0363248e8de253b7611e66b67eba45ce959

package/m*/Config.in: fix ordering of statements (2017-05-01 11:56:18 +0200)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to c85507a4104956f6597a910e729118309f3ff432

fs/ext2: add option to specify block size (2017-05-01 17:31:44 +0200)

----------------------------------------------------------------
J Evans (1):
ext2: add help text for BR2_TARGET_ROOTFS_EXT2_BLOCKS

Yann E. MORIN (8):
package/mke2img: check nb_blocks is specified package/mke2img: specifying zero inodes means auto fs/ext2: always pass the number of inodes fs/ext2: always pass the label option fs/ext2: allow reserving zero block for root fs/ext2: simplify code
package/mke2img: add option to specify block size fs/ext2: add option to specify block size

 fs/ext2/Config.in       | 35 +++++++++++++++++++++++++++++++++--
 fs/ext2/ext2.mk         | 27 +++++++++++----------------
 package/mke2img/mke2img | 15 ++++++++++++---
 3 files changed, 56 insertions(+), 21 deletions(-)

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2017-05-01 15:58 UTC #1 lore raw

PATCH 1/9 ext2: add help text for BR2_TARGET_ROOTFS_EXT2_BLOCKS

From: J Evans <[email protected]>

Signed-off-by J Evans <[email protected]>

Signed-off-by Yann E. MORIN <[email protected]>

---
 fs/ext2/Config.in | 2 ++
 1 file changed, 2 insertions(+)

fs/ext2/Config.in +2 -0

index 19ed140c88..a1e3647a14 100644
@@ -48,6 +48,8 @@ config BR2_TARGET_ROOTFS_EXT2_LABEL
 config BR2_TARGET_ROOTFS_EXT2_BLOCKS
 	int "exact size in blocks"
 	default 61440
+	help
+	  Specify the file system size as a number of 1024-byte blocks.
 
 config BR2_TARGET_ROOTFS_EXT2_INODES
 	int "exact number of inodes (leave at 0 for auto calculation)"

Signature · 1 line

2.11.0

Thomas Petazzoni

Applied

2017-05-04 15:15 UTC #2 lore raw

PATCH 1/9 ext2: add help text for BR2_TARGET_ROOTFS_EXT2_BLOCKS

Hello,

Quoted 7 lines · Yann E. MORIN

From: J Evans <[email protected]>
Signed-off-by J Evans <[email protected]>
Signed-off-by Yann E. MORIN <[email protected]>
---
 fs/ext2/Config.in | 2 ++
 1 file changed, 2 insertions(+)

Applied to master, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Yann E. MORIN

author

2017-05-01 15:58 UTC #3 lore raw

PATCH 2/9 package/mke2img: check nb_blocks is specified

In reply to Yann E. MORIN

Since we do not have autocalculation anymore, the user must specify the exact number of blocks.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/mke2img/mke2img | 3 +++
 1 file changed, 3 insertions(+)

package/mke2img/mke2img +3 -0

index b773aa99db..758b6dee68 100755
@@ -44,6 +44,9 @@ main() {
     if [ -z "${image}" ]; then
         error "you must specify an output image file with '-o'\n"
     fi
+    if [ -z "${nb_blocks}" ]; then
+        error "you must specify the size of the output image with '-b'\n"
+    fi
     case "${gen}:${rev}" in
     2:0|2:1|3:1|4:1)
         ;;

Signature · 1 line

2.11.0

Thomas Petazzoni

Applied

2017-05-13 13:17 UTC #4 lore raw

PATCH 2/9 package/mke2img: check nb_blocks is specified

Hello,

Quoted 7 lines · Yann E. MORIN

Since we do not have autocalculation anymore, the user must specify the exact number of blocks.
Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/mke2img/mke2img | 3 +++
 1 file changed, 3 insertions(+)

Applied to master, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Yann E. MORIN

author

2017-05-01 15:58 UTC #5 lore raw

PATCH 3/9 package/mke2img: specifying zero inodes means auto

In reply to Yann E. MORIN

Currently, leaving the number of inodes to be autocalculated requires the user to not specify the -i option at all.

Also accept zero as meaning auto; this will help simplify the ext2.mk code a little bit.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/mke2img/mke2img | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/mke2img/mke2img +1 -1

index 758b6dee68..266df483aa 100755
@@ -59,7 +59,7 @@ main() {
     esac
 
     # calculate needed inodes
-    if [ -z "${nb_inodes}" ]; then
+    if [ ${nb_inodes:-0} -eq 0 ]; then
         nb_inodes=$(find "${root_dir}" | wc -l)
         nb_inodes=$((nb_inodes+400))
     fi

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #6 lore raw

PATCH 4/9 fs/ext2: always pass the number of inodes

In reply to Yann E. MORIN

... since zero also means auto for the mke2img script, now.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/ext2.mk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

fs/ext2/ext2.mk +1 -2

index 9d3e8fd666..3fc12d7499 100644
@@ -8,9 +8,8 @@ EXT2_OPTS = -G $(BR2_TARGET_ROOTFS_EXT2_GEN) -R $(BR2_TARGET_ROOTFS_EXT2_REV)
 
 EXT2_OPTS += -b $(BR2_TARGET_ROOTFS_EXT2_BLOCKS)
 
-ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_INODES)),0)
 EXT2_OPTS += -i $(BR2_TARGET_ROOTFS_EXT2_INODES)
-endif
+
 EXT2_OPTS += -I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES)
 
 ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #7 lore raw

PATCH 5/9 fs/ext2: always pass the label option

In reply to Yann E. MORIN

... since passing an empty string is equivalent to not setting a label.

And fix the syntax highlighting in some editors...

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/ext2.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

fs/ext2/ext2.mk +2 -2

index 3fc12d7499..6a35b33117 100644
@@ -19,9 +19,9 @@ endif
 # qstrip results in stripping consecutive spaces into a single one. So the
 # variable is not qstrip-ed to preserve the integrity of the string value.
 EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
-ifneq ($(EXT2_LABEL),)
+#" Syntax highlighting... :-/ )
+
 EXT2_OPTS += -l "$(EXT2_LABEL)"
-endif
 
 ROOTFS_EXT2_DEPENDENCIES = host-mke2img

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #8 lore raw

PATCH 6/9 fs/ext2: allow reserving zero block for root

In reply to Yann E. MORIN

The previous default, zero, just meant "use the default value of the filesystem generator", which happened to be 5% (the traditional value for all ext-creating tools we've ever seen).

So, change the new default accordingly to 5%.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/Config.in | 7 ++++++-
 fs/ext2/ext2.mk   | 2 --
 2 files changed, 6 insertions(+), 3 deletions(-)

fs/ext2/Config.in +6 -1

index a1e3647a14..09ec87786f 100644
@@ -65,7 +65,12 @@ config BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES
 
 config BR2_TARGET_ROOTFS_EXT2_RESBLKS
 	int "reserved blocks percentage"
-	default 0
+	default 5
+	help
+	  The number of blocks on the filesystem (as a percentage of the
+	  total number of blocksi), that are reserved for use by root.
+	  Traditionally, this has been 5%, and all ext-related tools still
+	  default to reserving 5% when creating a nez ext filesystem.
 
 choice
 	prompt "Compression method"

fs/ext2/ext2.mk +0 -2

index 6a35b33117..57058b5fd2 100644
@@ -12,9 +12,7 @@ EXT2_OPTS += -i $(BR2_TARGET_ROOTFS_EXT2_INODES)
 
 EXT2_OPTS += -I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES)
 
-ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
 EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
-endif
 
 # qstrip results in stripping consecutive spaces into a single one. So the
 # variable is not qstrip-ed to preserve the integrity of the string value.

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #9 lore raw

PATCH 7/9 fs/ext2: simplify code

In reply to Yann E. MORIN

No option is conditional anymore, so just collate them into a single assignment.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/ext2.mk | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

fs/ext2/ext2.mk +8 ~~-11~~

index 57058b5fd2..16e14777c3 100644
@@ -4,22 +4,19 @@
 #
 ################################################################################
 
-EXT2_OPTS = -G $(BR2_TARGET_ROOTFS_EXT2_GEN) -R $(BR2_TARGET_ROOTFS_EXT2_REV)
-
-EXT2_OPTS += -b $(BR2_TARGET_ROOTFS_EXT2_BLOCKS)
-
-EXT2_OPTS += -i $(BR2_TARGET_ROOTFS_EXT2_INODES)
-
-EXT2_OPTS += -I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES)
-
-EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
-
 # qstrip results in stripping consecutive spaces into a single one. So the
 # variable is not qstrip-ed to preserve the integrity of the string value.
 EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
 #" Syntax highlighting... :-/ )
 
-EXT2_OPTS += -l "$(EXT2_LABEL)"
+EXT2_OPTS = \
+	-G $(BR2_TARGET_ROOTFS_EXT2_GEN) \
+	-R $(BR2_TARGET_ROOTFS_EXT2_REV) \
+	-b $(BR2_TARGET_ROOTFS_EXT2_BLOCKS) \
+	-i $(BR2_TARGET_ROOTFS_EXT2_INODES) \
+	-I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES) \
+	-r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS) \
+	-l "$(EXT2_LABEL)"
 
 ROOTFS_EXT2_DEPENDENCIES = host-mke2img

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #10 lore raw

PATCH 8/9 package/mke2img: add option to specify block size

In reply to Yann E. MORIN

ext filesystems can use a block size of 1024, 2048, or 4096 bytes, the former being interesting to store small files, while the latter being more intersting to store bigger files.

So far, we were using the defualt, which was to use a 1024-byte block. Continue doing so (for now...).

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/ext2.mk         |  1 +
 package/mke2img/mke2img | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

fs/ext2/ext2.mk +1 -0

index 16e14777c3..0d5fc7f118 100644
@@ -12,6 +12,7 @@ EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
 EXT2_OPTS = \
 	-G $(BR2_TARGET_ROOTFS_EXT2_GEN) \
 	-R $(BR2_TARGET_ROOTFS_EXT2_REV) \
+	-B 1024 \
 	-b $(BR2_TARGET_ROOTFS_EXT2_BLOCKS) \
 	-i $(BR2_TARGET_ROOTFS_EXT2_INODES) \
 	-I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES) \

package/mke2img/mke2img +8 -2

index 266df483aa..399012a2be 100755
@@ -19,9 +19,10 @@ main() {
     rev=1
     nb_extra_inodes=0
 
-    while getopts :hb:i:I:r:d:o:G:R:l:u: OPT; do
+    while getopts :hB:b:i:I:r:d:o:G:R:l:u: OPT; do
         case "${OPT}" in
         h)  help; exit 0;;
+        B)  block_size="${OPTARG}";;
         b)  nb_blocks=${OPTARG};;
         i)  nb_inodes=${OPTARG};;
         I)  nb_extra_inodes=${OPTARG};;

@@ -47,6 +48,11 @@ main() {
     if [ -z "${nb_blocks}" ]; then
         error "you must specify the size of the output image with '-b'\n"
     fi
+    case "${block_size}" in
+    1024|2048|4096) ;; # Sole valid values
+    '') error "you must specify the size of a block with -B\n";;
+    *)  error "invalid block size '%s' (valid: 1024. 2048, 4096)\n" "${block_size}";;
+    esac
     case "${gen}:${rev}" in
     2:0|2:1|3:1|4:1)
         ;;

@@ -91,7 +97,7 @@ main() {
     fi
 
     # Generate the filesystem
-    genext2fs_opts=( -z -b ${nb_blocks} -N ${nb_inodes} -d "${root_dir}" )
+    genext2fs_opts=( -z -B ${block_size} -b ${nb_blocks} -N ${nb_inodes} -d "${root_dir}" )
     if [ -n "${nb_res_blocks}" ]; then
         genext2fs_opts+=( -m ${nb_res_blocks} )
     fi

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-05-01 15:58 UTC #11 lore raw

PATCH 9/9 fs/ext2: add option to specify block size

In reply to Yann E. MORIN

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 fs/ext2/Config.in | 28 ++++++++++++++++++++++++++--
 fs/ext2/ext2.mk   |  2 +-
 2 files changed, 27 insertions(+), 3 deletions(-)

fs/ext2/Config.in +26 -2

index 09ec87786f..627294e17f 100644
@@ -44,12 +44,36 @@ config BR2_TARGET_ROOTFS_EXT2_REV
 config BR2_TARGET_ROOTFS_EXT2_LABEL
 	string "filesystem label"
 
-# 61440 = 60MB, i.e usually small enough to fit on a 64MB media
+choice
+	bool "block size"
+	help
+	  The size, in bytes, of a block.
+
+config BR2_TARGET_ROOTFS_EXT2_BLOCK_1024
+	bool "1024"
+
+config BR2_TARGET_ROOTFS_EXT2_BLOCK_2048
+	bool "2048"
+
+config BR2_TARGET_ROOTFS_EXT2_BLOCK_4096
+	bool "4096"
+
+endchoice
+
+config BR2_TARGET_ROOTFS_EXT2_BLOCK_SIZE
+	int
+	default 1024 if BR2_TARGET_ROOTFS_EXT2_BLOCK_1024
+	default 2048 if BR2_TARGET_ROOTFS_EXT2_BLOCK_2048
+	default 4096 if BR2_TARGET_ROOTFS_EXT2_BLOCK_4096
+
+# 61440 block od 1024 bytes = 60MB, i.e usually small enough to fit
+# on a 64MB media
 config BR2_TARGET_ROOTFS_EXT2_BLOCKS
 	int "exact size in blocks"
 	default 61440
 	help
-	  Specify the file system size as a number of 1024-byte blocks.
+	  Specify the file system size as a number of blocks, which
+	  size is specified above.
 
 config BR2_TARGET_ROOTFS_EXT2_INODES
 	int "exact number of inodes (leave at 0 for auto calculation)"

fs/ext2/ext2.mk +1 -1

index 0d5fc7f118..ec49caa3a5 100644
@@ -12,7 +12,7 @@ EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
 EXT2_OPTS = \
 	-G $(BR2_TARGET_ROOTFS_EXT2_GEN) \
 	-R $(BR2_TARGET_ROOTFS_EXT2_REV) \
-	-B 1024 \
+	-B $(BR2_TARGET_ROOTFS_EXT2_BLOCK_SIZE) \
 	-b $(BR2_TARGET_ROOTFS_EXT2_BLOCKS) \
 	-i $(BR2_TARGET_ROOTFS_EXT2_INODES) \
 	-I $(BR2_TARGET_ROOTFS_EXT2_EXTRA_INODES) \

Signature · 1 line

2.11.0

Thomas Petazzoni

2017-05-01 19:18 UTC #12 lore raw

cover fs/ext2: cleanups and improvements

In reply to Yann E. MORIN

Hello,

Quoted 12 lines · Yann E. MORIN

J Evans (1):
ext2: add help text for BR2_TARGET_ROOTFS_EXT2_BLOCKS
Yann E. MORIN (8):
package/mke2img: check nb_blocks is specified package/mke2img: specifying zero inodes means auto fs/ext2: always pass the number of inodes fs/ext2: always pass the label option fs/ext2: allow reserving zero block for root fs/ext2: simplify code
package/mke2img: add option to specify block size fs/ext2: add option to specify block size

In general, I find this good. However, it badly conflicts with the on-going work to move away from mke2img/genext2fs and use mkfs.ext2/3/4 instead.

For example, I believe mkfs.ext2/3/4 will automatically chose a sane default block size depending on the filesystem size.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Yann E. MORIN

author

2017-05-01 20:58 UTC #13 lore raw

cover fs/ext2: cleanups and improvements

Thomas, All,

On 2017-05-01 21:18 +0200, Thomas Petazzoni spake thusly:

Quoted 14 lines · Yann E. MORIN

Quoted 12 lines
J Evans (1):
ext2: add help text for BR2_TARGET_ROOTFS_EXT2_BLOCKS
Yann E. MORIN (8):
package/mke2img: check nb_blocks is specified package/mke2img: specifying zero inodes means auto fs/ext2: always pass the number of inodes fs/ext2: always pass the label option fs/ext2: allow reserving zero block for root fs/ext2: simplify code
package/mke2img: add option to specify block size fs/ext2: add option to specify block size

Quoted 3 lines

In general, I find this good. However, it badly conflicts with the on-going work to move away from mke2img/genext2fs and use mkfs.ext2/3/4 instead.

But this is not ready yet, is it?

Quoted 2 lines

For example, I believe mkfs.ext2/3/4 will automatically chose a sane default block size depending on the filesystem size.

And even if it does, ther are stil cases where one wants to override the auto-calculated size.

So, even when we switch the backend that generates the filesystem, we'd still want to have those options, I believe.

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'