mkfs.ubifs: Only require 17 LEBs

Message ID 20170705163411.23546-1-romain.izard.pro@gmail.com
State New
Delegated to: David Oberhollenzer
Headers show

Commit Message

Romain Izard July 5, 2017, 4:34 p.m.
The minimum size for a UBIFS volume is 17 LEBs. As mkfs.ubifs counted
the reserved log and orphan blocks twice, the program did not allow
such a small volume.

Fix the calculations to be able to build an image with 17 LEBs. With
this, the following command works:

mkfs.ubifs -c 17 -l2 -m 2048 -e 124KiB -o small.ubifs

Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Artem Bityutskiy July 6, 2017, 6:46 a.m. | #1
If this change is done in userspace, it should also be done in the
kernel, which also can "format" the media.

I cannot remember now, but very small volumes were very problematic.
May be it was very inprecise free space estimation. Or something like
-ENOSPC when trying to remove a file.

The point is, make sure to test the tiny volumes very thoroughly before
this is merged.

On Wed, 2017-07-05 at 18:34 +0200, Romain Izard wrote:
> The minimum size for a UBIFS volume is 17 LEBs. As mkfs.ubifs counted
> the reserved log and orphan blocks twice, the program did not allow
> such a small volume.
> 
> Fix the calculations to be able to build an image with 17 LEBs. With
> this, the following command works:
> 
> mkfs.ubifs -c 17 -l2 -m 2048 -e 124KiB -o small.ubifs
> 
> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
> ---
>  ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-
> utils/mkfs.ubifs/mkfs.ubifs.c
> index 9e69a4f..ba0293c 100644
> --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> @@ -331,6 +331,9 @@ static long long add_space_overhead(long long
> size)
>          return size / divisor;
>  }
>  
> +#define UBIFS_MIN_LEB_NOLOG   (UBIFS_MIN_LEB_CNT -
> UBIFS_MIN_LOG_LEBS)
> +#define UBIFS_MIN_LEB_NOORPH  (UBIFS_MIN_LEB_CNT -
> UBIFS_MIN_ORPH_LEBS)
> +
>  static int validate_options(void)
>  {
>  	int tmp;
> @@ -372,15 +375,15 @@ static int validate_options(void)
>  	if (c->log_lebs < UBIFS_MIN_LOG_LEBS)
>  		return err_msg("too few log LEBs, minimum is %d",
>  			       UBIFS_MIN_LOG_LEBS);
> -	if (c->log_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT)
> +	if (c->log_lebs > c->max_leb_cnt - UBIFS_MIN_LEB_NOLOG)
>  		return err_msg("too many log LEBs, maximum is %d",
> -			       c->max_leb_cnt - UBIFS_MIN_LEB_CNT);
> +			       c->max_leb_cnt -
> UBIFS_MIN_LEB_NOLOG);
>  	if (c->orph_lebs < UBIFS_MIN_ORPH_LEBS)
>  		return err_msg("too few orphan LEBs, minimum is %d",
>  			       UBIFS_MIN_ORPH_LEBS);
> -	if (c->orph_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT)
> +	if (c->orph_lebs > c->max_leb_cnt - UBIFS_MIN_LEB_NOORPH)
>  		return err_msg("too many orphan LEBs, maximum is
> %d",
> -			       c->max_leb_cnt - UBIFS_MIN_LEB_CNT);
> +			       c->max_leb_cnt -
> UBIFS_MIN_LEB_NOORPH);
>  	tmp = UBIFS_SB_LEBS + UBIFS_MST_LEBS + c->log_lebs + c-
> >lpt_lebs;
>  	tmp += c->orph_lebs + 4;
>  	if (tmp > c->max_leb_cnt)
Romain Izard July 6, 2017, 7:57 a.m. | #2
2017-07-06 8:46 GMT+02:00 Artem Bityutskiy <dedekind1@gmail.com>:
>
> If this change is done in userspace, it should also be done in the
> kernel, which also can "format" the media.
>
> I cannot remember now, but very small volumes were very problematic.
> May be it was very inprecise free space estimation. Or something like
> -ENOSPC when trying to remove a file.
>
> The point is, make sure to test the tiny volumes very thoroughly before
> this is merged.
>

The algorithm used to determine the size of a UBIFS volume is not
exactly the same in mkfs.ubifs and in the kernel. mkfs.ubifs lets the
user tune more parameters, by specifying the number of log LEBs or
orphan LEBs, whereas the kernel calculates all these values by itself.

As a result, the kernel already accepts such small file systems, and
creates them when trying to mount an empty UBI volume of 17 LEBs. If the
empty volume has 16 or less LEBs, mounting fails with the following
error message:

[66897.450000] UBIFS error (ubi0:12 pid 12977): ubifs_mount: too few
LEBs (16), min. is 17

I modified mkfs.ubifs to be able to generate offline an image matching
the exact setup of an existing device. In this device, there is a small
volume with a tiny 17 LEBs UBIFS. I did not notice anything special
about it, but your comment worries me a little as I do not really have
the means to test "very thoroughly" this configuration.

Should I do something about it ?

Best regards,

Patch

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 9e69a4f..ba0293c 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -331,6 +331,9 @@  static long long add_space_overhead(long long size)
         return size / divisor;
 }
 
+#define UBIFS_MIN_LEB_NOLOG   (UBIFS_MIN_LEB_CNT - UBIFS_MIN_LOG_LEBS)
+#define UBIFS_MIN_LEB_NOORPH  (UBIFS_MIN_LEB_CNT - UBIFS_MIN_ORPH_LEBS)
+
 static int validate_options(void)
 {
 	int tmp;
@@ -372,15 +375,15 @@  static int validate_options(void)
 	if (c->log_lebs < UBIFS_MIN_LOG_LEBS)
 		return err_msg("too few log LEBs, minimum is %d",
 			       UBIFS_MIN_LOG_LEBS);
-	if (c->log_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT)
+	if (c->log_lebs > c->max_leb_cnt - UBIFS_MIN_LEB_NOLOG)
 		return err_msg("too many log LEBs, maximum is %d",
-			       c->max_leb_cnt - UBIFS_MIN_LEB_CNT);
+			       c->max_leb_cnt - UBIFS_MIN_LEB_NOLOG);
 	if (c->orph_lebs < UBIFS_MIN_ORPH_LEBS)
 		return err_msg("too few orphan LEBs, minimum is %d",
 			       UBIFS_MIN_ORPH_LEBS);
-	if (c->orph_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT)
+	if (c->orph_lebs > c->max_leb_cnt - UBIFS_MIN_LEB_NOORPH)
 		return err_msg("too many orphan LEBs, maximum is %d",
-			       c->max_leb_cnt - UBIFS_MIN_LEB_CNT);
+			       c->max_leb_cnt - UBIFS_MIN_LEB_NOORPH);
 	tmp = UBIFS_SB_LEBS + UBIFS_MST_LEBS + c->log_lebs + c->lpt_lebs;
 	tmp += c->orph_lebs + 4;
 	if (tmp > c->max_leb_cnt)