| Message ID | 20231229135653.4764-1-maukka@ext.kapsi.fi |
|---|---|
| State | Rejected |
| Delegated to: | David Oberhollenzer |
| Headers | show |
| Series | mkfs.ubifs: Add option to minimize the amount of LEBs | expand |
On 29.12.2023 15.56, Mauri Sandberg wrote: > Use case for the new option would be when you want to have a > non-mutable filesystem in a static ubifs volume and another > volume for writable data. > > Currently you can find the minimal LEB count by running the > mkfs.ubifs once with too small max_leb_cnt. This patch uses > the already calculated minimal amount of LEBs as the max_leb_cnt > and then proceeds to create the image as usual. > > Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> > --- > ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 40 +++++++++++++++++++++-------- > 1 file changed, 30 insertions(+), 10 deletions(-) > > diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c > index 8f8d40b..02ca337 100644 > --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c > +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c > @@ -141,6 +141,7 @@ static int do_create_inum_attr; > static char *context; > static int context_len; > static struct stat context_st; > +static int minimize; > > /* The 'head' (position) which nodes are written */ > static int head_lnum; > @@ -163,7 +164,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:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQqaK:b:P:C:"; > +static const char *optstring = "d:r:m:o:D:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQqaK:b:P:C:M"; > > enum { > HASH_ALGO_OPTION = CHAR_MAX + 1, > @@ -202,6 +203,7 @@ static const struct option longopts[] = { > {"hash-algo", 1, NULL, HASH_ALGO_OPTION}, > {"auth-key", 1, NULL, AUTH_KEY_OPTION}, > {"auth-cert", 1, NULL, AUTH_CERT_OPTION}, > + {"minimize-lebs", 0, NULL, 'M'}, > {NULL, 0, NULL, 0} > }; > > @@ -258,6 +260,7 @@ static const char *helptext = > " for signing\n" > " --auth-cert=FILE Authentication certificate filename for signing. Unused\n" > " when certificate is provided via PKCS #11\n" > +"-M --minimize-lebs use minimal amount of LEBs\n" > "-h, --help display this help text\n\n" > "Note, SIZE is specified in bytes, but it may also be specified in Kilobytes,\n" > "Megabytes, and Gigabytes if a KiB, MiB, or GiB suffix is used.\n\n" > @@ -420,8 +423,11 @@ static int validate_options(void) > return err_msg("too large LEB size %d, maximum is %d", > c->leb_size, UBIFS_MAX_LEB_SZ); > if (c->max_leb_cnt < UBIFS_MIN_LEB_CNT) > - return err_msg("too low max. count of LEBs, minimum is %d", > - UBIFS_MIN_LEB_CNT); > + if (minimize) > + c->max_leb_cnt = UBIFS_MIN_LEB_CNT; > + else > + return err_msg("too low max. count of LEBs, minimum is %d", > + UBIFS_MIN_LEB_CNT); > if (c->fanout < UBIFS_MIN_FANOUT) > return err_msg("too low fanout, minimum is %d", > UBIFS_MIN_FANOUT); > @@ -432,13 +438,13 @@ 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 (!minimize && c->log_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT) > return err_msg("too many log LEBs, maximum is %d", > c->max_leb_cnt - UBIFS_MIN_LEB_CNT); > 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 (!minimize && c->orph_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT) > return err_msg("too many orphan LEBs, maximum is %d", > c->max_leb_cnt - UBIFS_MIN_LEB_CNT); > tmp = UBIFS_SB_LEBS + UBIFS_MST_LEBS + c->log_lebs + c->lpt_lebs; > @@ -800,6 +806,9 @@ static int get_options(int argc, char**argv) > case AUTH_CERT_OPTION: > return err_msg("mkfs.ubifs was built without crypto support."); > #endif > + case 'M': > + minimize = 1; > + break; > } > } > > @@ -847,8 +856,12 @@ static int get_options(int argc, char**argv) > if (c->leb_size == -1) > return err_msg("LEB size was not specified (use -h for help)"); > > - if (c->max_leb_cnt == -1) > - return err_msg("Maximum count of LEBs was not specified " > + if (!minimize && c->max_leb_cnt == -1) > + return err_msg("Maximum count of LEBs or --minimize-lebs was not specified " > + "(use -h for help)"); > + > + if (minimize && c->max_leb_cnt != -1) > + return err_msg("You have to specify either --minimize-lebs or --max-leb-cnt " > "(use -h for help)"); > > if (c->max_bud_bytes == -1) { > @@ -876,7 +889,8 @@ static int get_options(int argc, char**argv) > > if (c->log_lebs == -1) { > c->log_lebs = calc_min_log_lebs(c->max_bud_bytes); > - c->log_lebs += 2; > + if (!minimize) > + c->log_lebs += 2; > } > > if (c->min_io_size < 8) > @@ -888,7 +902,10 @@ static int get_options(int argc, char**argv) > printf("\troot: %s\n", root); > printf("\tmin_io_size: %d\n", c->min_io_size); > printf("\tleb_size: %d\n", c->leb_size); > - printf("\tmax_leb_cnt: %d\n", c->max_leb_cnt); > + if (minimize) > + printf("\tminimize: %d\n", minimize); > + else > + printf("\tmax_leb_cnt: %d\n", c->max_leb_cnt); > printf("\toutput: %s\n", output); > printf("\tjrn_size: %llu\n", c->max_bud_bytes); > printf("\treserved: %llu\n", c->rp_size); > @@ -2553,7 +2570,10 @@ static int finalize_leb_cnt(void) > { > c->leb_cnt = head_lnum; > if (c->leb_cnt > c->max_leb_cnt) > - return err_msg("max_leb_cnt too low (%d needed)", c->leb_cnt); > + if (minimize) > + c->max_leb_cnt = c->leb_cnt; > + else > + return err_msg("max_leb_cnt too low (%d needed)", c->leb_cnt); > c->main_lebs = c->leb_cnt - c->main_first; > if (verbose) { > printf("\tsuper lebs: %d\n", UBIFS_SB_LEBS); > > base-commit: 9471c13faf76ff05f58d636b988bb066ad6d05fa Friendly ping. Also bringing in Richard and David as I thought at least they could be relevant maintainers. Thanks.
----- Ursprüngliche Mail ----- > Von: "Mauri Sandberg" <maukka@ext.kapsi.fi> >> Use case for the new option would be when you want to have a >> non-mutable filesystem in a static ubifs volume and another >> volume for writable data. >> >> Currently you can find the minimal LEB count by running the >> mkfs.ubifs once with too small max_leb_cnt. This patch uses >> the already calculated minimal amount of LEBs as the max_leb_cnt >> and then proceeds to create the image as usual. [...] > Friendly ping. Also bringing in Richard and David as I thought at least > they could be relevant maintainers. Thanks. Seems like a legit feature. But what will happen if somebody tries to use a minimized UBIFS in rw-mode? I fear then UBIFS will fail because it has not enough log LEBs or such? Thanks, //richard
Thanks for your propmpt response, Richard. On 6.1.2024 12.16, Richard Weinberger wrote: > But what will happen if somebody tries to > use a minimized UBIFS in rw-mode? > I fear then UBIFS will fail because it has not enough log LEBs or such? First off I would assume it would mount the filesystem writable with very little free space available. Here I am assuming the mininum LEB count mkfs.ubifs calculates is legit and that the same goes for the journal and log sizes. Also, my current understanding is that the LEB calculation is based on uncompressed size. If ubi uses compression runtime then the amount of LEBs actually used is smaller, no? I'll have to do some tests on the topic and see how it behaves. I'll get back to you.
----- Ursprüngliche Mail ----- > Von: "Mauri Sandberg" <maukka@ext.kapsi.fi> > An: "richard" <richard@nod.at> > CC: "Mauri Sandberg" <maukka@ext.kapsi.fi>, "david oberhollenzer" <david.oberhollenzer@sigma-star.at>, "linux-mtd" > <linux-mtd@lists.infradead.org> > Gesendet: Samstag, 6. Januar 2024 15:46:11 > Betreff: Re: [PATCH] mkfs.ubifs: Add option to minimize the amount of LEBs > Thanks for your propmpt response, Richard. > > On 6.1.2024 12.16, Richard Weinberger wrote: >> But what will happen if somebody tries to >> use a minimized UBIFS in rw-mode? >> I fear then UBIFS will fail because it has not enough log LEBs or such? > > First off I would assume it would mount the filesystem writable with > very little free space available. Here I am assuming the mininum LEB > count mkfs.ubifs calculates is legit and that the same goes for the > journal and log sizes. Hm, I gave your patch a try. As soon I use "-M", the resulting file system does not mount. No matter how large the UBI volume is. [18034.687392] UBIFS (ubi0:0): Mounting in unauthenticated mode [18034.688241] UBIFS (ubi0:0): background thread "ubifs_bgt0_0" started, PID 1428 [18034.688484] UBIFS error (ubi0:0 pid 1427): check_lpt_type.constprop.19: invalid type (0) in LPT node type 1 [18034.690139] CPU: 2 PID: 1427 Comm: mount Not tainted 6.7.0-rc7-00004-gc07a4dab243a #246 [18034.690871] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552c-rebuilt.opensuse.org 04/01/2014 [18034.690873] Call Trace: [18034.690886] <TASK> [18034.690889] dump_stack_lvl+0x32/0x50 [18034.690899] check_lpt_type.constprop.19+0x36/0x40 [18034.690905] ubifs_unpack_nnode+0x4a/0x120 [18034.690910] ubifs_read_nnode+0x1bf/0x210 [18034.690913] ubifs_lpt_lookup_dirty+0x165/0x2e0 [18034.690916] ? leb_write_unlock+0x72/0xc0 [18034.690920] ubifs_replay_journal+0x44/0x11d0 [18034.690922] ? next_sqnum+0x27/0x90 [18034.690926] ? ubifs_leb_write+0x5f/0x100 [18034.695675] ? ubifs_write_node_hmac+0xcd/0x1d0 [18034.695679] ubifs_mount+0x1305/0x17d0 [18034.695682] ? __pfx_bud_wbuf_callback+0x10/0x10 [18034.695684] ? legacy_get_tree+0x22/0x40 [18034.695688] ? __pfx_ubifs_mount+0x10/0x10 [18034.695690] legacy_get_tree+0x22/0x40 [18034.695692] vfs_get_tree+0x20/0xd0 [18034.695695] path_mount+0x59c/0x9a0 [18034.695698] ? user_path_at_empty+0x3b/0x50 [18034.695702] do_mount+0x74/0x90 [18034.695704] __x64_sys_mount+0x81/0xe0 [18034.695706] do_syscall_64+0x42/0xf0 [18034.695711] entry_SYSCALL_64_after_hwframe+0x6f/0x77 > Also, my current understanding is that the LEB calculation is based on > uncompressed size. If ubi uses compression runtime then the amount of > LEBs actually used is smaller, no? I don't think this matters much here. The LEB calculation in mkfs is to make sure UBIFS has enough space on the resulting UBI volume to work correctly. E.g. Space for the log, etc.. > I'll have to do some tests on the topic and see how it behaves. I'll get > back to you. Ok! Thanks, //richard
On 6.1.2024 17.23, Richard Weinberger wrote: > Hm, I gave your patch a try. As soon I use "-M", the resulting file system > does not mount. No matter how large the UBI volume is. You are most correct here, it does not mount nicely with the '-M'. On the other hand the minimal filesystem produced with, say, '-c XX and -l 2' does mount nicely as rw too. So in theory this could work, the solution just was not that simple. I'll approach you with another version should I get it working. Thanks for your time. :)
----- Ursprüngliche Mail ----- > Von: "Mauri Sandberg" <maukka@ext.kapsi.fi> > An: "richard" <richard@nod.at> > CC: "Mauri Sandberg" <maukka@ext.kapsi.fi>, "david oberhollenzer" <david.oberhollenzer@sigma-star.at>, "linux-mtd" > <linux-mtd@lists.infradead.org> > Gesendet: Sonntag, 7. Januar 2024 08:53:05 > Betreff: Re: [PATCH] mkfs.ubifs: Add option to minimize the amount of LEBs > On 6.1.2024 17.23, Richard Weinberger wrote: >> Hm, I gave your patch a try. As soon I use "-M", the resulting file system >> does not mount. No matter how large the UBI volume is. > > You are most correct here, it does not mount nicely with the '-M'. On > the other hand the minimal filesystem produced with, say, '-c XX and -l > 2' does mount nicely as rw too. So in theory this could work, the > solution just was not that simple. Huh? I assumed the patch you sent actually works. At least for your use case... Please don't send untested material. Thanks, //richard
diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c index 8f8d40b..02ca337 100644 --- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c +++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c @@ -141,6 +141,7 @@ static int do_create_inum_attr; static char *context; static int context_len; static struct stat context_st; +static int minimize; /* The 'head' (position) which nodes are written */ static int head_lnum; @@ -163,7 +164,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:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQqaK:b:P:C:"; +static const char *optstring = "d:r:m:o:D:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQqaK:b:P:C:M"; enum { HASH_ALGO_OPTION = CHAR_MAX + 1, @@ -202,6 +203,7 @@ static const struct option longopts[] = { {"hash-algo", 1, NULL, HASH_ALGO_OPTION}, {"auth-key", 1, NULL, AUTH_KEY_OPTION}, {"auth-cert", 1, NULL, AUTH_CERT_OPTION}, + {"minimize-lebs", 0, NULL, 'M'}, {NULL, 0, NULL, 0} }; @@ -258,6 +260,7 @@ static const char *helptext = " for signing\n" " --auth-cert=FILE Authentication certificate filename for signing. Unused\n" " when certificate is provided via PKCS #11\n" +"-M --minimize-lebs use minimal amount of LEBs\n" "-h, --help display this help text\n\n" "Note, SIZE is specified in bytes, but it may also be specified in Kilobytes,\n" "Megabytes, and Gigabytes if a KiB, MiB, or GiB suffix is used.\n\n" @@ -420,8 +423,11 @@ static int validate_options(void) return err_msg("too large LEB size %d, maximum is %d", c->leb_size, UBIFS_MAX_LEB_SZ); if (c->max_leb_cnt < UBIFS_MIN_LEB_CNT) - return err_msg("too low max. count of LEBs, minimum is %d", - UBIFS_MIN_LEB_CNT); + if (minimize) + c->max_leb_cnt = UBIFS_MIN_LEB_CNT; + else + return err_msg("too low max. count of LEBs, minimum is %d", + UBIFS_MIN_LEB_CNT); if (c->fanout < UBIFS_MIN_FANOUT) return err_msg("too low fanout, minimum is %d", UBIFS_MIN_FANOUT); @@ -432,13 +438,13 @@ 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 (!minimize && c->log_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT) return err_msg("too many log LEBs, maximum is %d", c->max_leb_cnt - UBIFS_MIN_LEB_CNT); 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 (!minimize && c->orph_lebs >= c->max_leb_cnt - UBIFS_MIN_LEB_CNT) return err_msg("too many orphan LEBs, maximum is %d", c->max_leb_cnt - UBIFS_MIN_LEB_CNT); tmp = UBIFS_SB_LEBS + UBIFS_MST_LEBS + c->log_lebs + c->lpt_lebs; @@ -800,6 +806,9 @@ static int get_options(int argc, char**argv) case AUTH_CERT_OPTION: return err_msg("mkfs.ubifs was built without crypto support."); #endif + case 'M': + minimize = 1; + break; } } @@ -847,8 +856,12 @@ static int get_options(int argc, char**argv) if (c->leb_size == -1) return err_msg("LEB size was not specified (use -h for help)"); - if (c->max_leb_cnt == -1) - return err_msg("Maximum count of LEBs was not specified " + if (!minimize && c->max_leb_cnt == -1) + return err_msg("Maximum count of LEBs or --minimize-lebs was not specified " + "(use -h for help)"); + + if (minimize && c->max_leb_cnt != -1) + return err_msg("You have to specify either --minimize-lebs or --max-leb-cnt " "(use -h for help)"); if (c->max_bud_bytes == -1) { @@ -876,7 +889,8 @@ static int get_options(int argc, char**argv) if (c->log_lebs == -1) { c->log_lebs = calc_min_log_lebs(c->max_bud_bytes); - c->log_lebs += 2; + if (!minimize) + c->log_lebs += 2; } if (c->min_io_size < 8) @@ -888,7 +902,10 @@ static int get_options(int argc, char**argv) printf("\troot: %s\n", root); printf("\tmin_io_size: %d\n", c->min_io_size); printf("\tleb_size: %d\n", c->leb_size); - printf("\tmax_leb_cnt: %d\n", c->max_leb_cnt); + if (minimize) + printf("\tminimize: %d\n", minimize); + else + printf("\tmax_leb_cnt: %d\n", c->max_leb_cnt); printf("\toutput: %s\n", output); printf("\tjrn_size: %llu\n", c->max_bud_bytes); printf("\treserved: %llu\n", c->rp_size); @@ -2553,7 +2570,10 @@ static int finalize_leb_cnt(void) { c->leb_cnt = head_lnum; if (c->leb_cnt > c->max_leb_cnt) - return err_msg("max_leb_cnt too low (%d needed)", c->leb_cnt); + if (minimize) + c->max_leb_cnt = c->leb_cnt; + else + return err_msg("max_leb_cnt too low (%d needed)", c->leb_cnt); c->main_lebs = c->leb_cnt - c->main_first; if (verbose) { printf("\tsuper lebs: %d\n", UBIFS_SB_LEBS);
Use case for the new option would be when you want to have a non-mutable filesystem in a static ubifs volume and another volume for writable data. Currently you can find the minimal LEB count by running the mkfs.ubifs once with too small max_leb_cnt. This patch uses the already calculated minimal amount of LEBs as the max_leb_cnt and then proceeds to create the image as usual. Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi> --- ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 40 +++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) base-commit: 9471c13faf76ff05f58d636b988bb066ad6d05fa