From 184cf99a351b46f5f70353b5bffa623b84836fa1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 16:24:22 +0100 Subject: [PATCH 1/5] repart: use LESS_BY() more --- src/partition/repart.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 62cc8ff7ae..482ef156ef 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -528,16 +528,9 @@ static int free_area_compare(FreeArea *const *a, FreeArea *const*b) { } static uint64_t charge_size(uint64_t total, uint64_t amount) { - uint64_t rounded; - - assert(amount <= total); - /* Subtract the specified amount from total, rounding up to multiple of 4K if there's room */ - rounded = round_up_size(amount, 4096); - if (rounded >= total) - return 0; - - return total - rounded; + assert(amount <= total); + return LESS_BY(total, round_up_size(amount, 4096)); } static uint64_t charge_weight(uint64_t total, uint64_t amount) { @@ -1491,11 +1484,7 @@ static int determine_current_padding( offset = round_up_size(offset, 4096); next = round_down_size(next, 4096); - if (next >= offset) /* Check again, rounding might have fucked things up */ - *ret = next - offset; - else - *ret = 0; - + *ret = LESS_BY(next, offset); /* Saturated substraction, rounding might have fucked things up */ return 0; } From ae0613c6c4cf8aa2d924c9468e8dac1e792a4e71 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 16:25:00 +0100 Subject: [PATCH 2/5] repart: simplify stat machine we mostly go through linearly --- src/partition/repart.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 482ef156ef..db2f95afc7 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -644,6 +644,8 @@ typedef enum GrowPartitionPhase { /* The third phase: we distribute what remains among the remaining partitions, according to the weights */ PHASE_DISTRIBUTE, + + _GROW_PARTITION_PHASE_MAX, } GrowPartitionPhase; static int context_grow_partitions_phase( @@ -779,20 +781,14 @@ static int context_grow_partitions_on_free_area(Context *context, FreeArea *a) { span += round_up_size(a->after->offset + a->after->current_size, 4096) - a->after->offset; } - GrowPartitionPhase phase = PHASE_OVERCHARGE; - for (;;) { + for (GrowPartitionPhase phase = 0; phase < _GROW_PARTITION_PHASE_MAX;) { r = context_grow_partitions_phase(context, a, phase, &span, &weight_sum); if (r < 0) return r; if (r == 0) /* not done yet, re-run this phase */ continue; - if (phase == PHASE_OVERCHARGE) - phase = PHASE_UNDERCHARGE; - else if (phase == PHASE_UNDERCHARGE) - phase = PHASE_DISTRIBUTE; - else if (phase == PHASE_DISTRIBUTE) - break; + phase++; /* got to next phase */ } /* We still have space left over? Donate to preceding partition if we have one */ From 0b7f574f72c72afb9384d7ea11b324ead067c8d1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 16:25:24 +0100 Subject: [PATCH 3/5] repart: don't distribute space after unaligned partitions If we operate on a disk that has a pre-existing unaligned partition (i.e. one that doesn't start on multiple of 4K, or doesn't have a size of multiple 4K), then the amount of space after it to distribute among partitions isn't a multiple of 4K either. So far we might end up passing the remaining fraction to any partition that wanted it, which was usually the first one after it that is newly defined. This then confused the later placement algorithm, since it assumed all partitions we newly allocate were properly aligned but by being extended by the fractional space they wouldn't be anymore. Let's hence fix that by ensuring we never pass space to later partitions so that things wouldn't be aligned anymore. Anything that is left-over then at the very end (i.e. typically exactly the remaining fraction) is added as padding to the existing, unaligned partition, so that it can't confuse anyone. Fixes: #20622 --- src/partition/repart.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index db2f95afc7..3ff9752005 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -796,7 +796,9 @@ static int context_grow_partitions_on_free_area(Context *context, FreeArea *a) { uint64_t m, xsz; assert(a->after->new_size != UINT64_MAX); - m = a->after->new_size + span; + + /* Calculate new size and align (but ensure this doesn't shrink the size) */ + m = MAX(a->after->new_size, round_down_size(a->after->new_size + span, 4096)); xsz = partition_max_size(a->after); if (xsz != UINT64_MAX && m > xsz) @@ -821,7 +823,7 @@ static int context_grow_partitions_on_free_area(Context *context, FreeArea *a) { continue; assert(p->new_size != UINT64_MAX); - m = p->new_size + span; + m = MAX(p->new_size, round_down_size(p->new_size + span, 4096)); xsz = partition_max_size(p); if (xsz != UINT64_MAX && m > xsz) @@ -4973,7 +4975,7 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; - /* Now calculate where each partition gets placed */ + /* Now calculate where each new partition gets placed */ context_place_partitions(context); /* Make sure each partition has a unique UUID and unique label */ From 1052a1142d8328a77bdc5011e749619f8e73ed54 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 16:31:48 +0100 Subject: [PATCH 4/5] repart: fix free area calculations for unaligned partitions To properly detect how much space we have to distribute we need to take into account that both the partition offset and the partition size aren't aligned. --- src/partition/repart.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/partition/repart.c b/src/partition/repart.c index 3ff9752005..5e6e88df94 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -505,18 +505,21 @@ static uint64_t free_area_available_for_new_partitions(const FreeArea *a) { avail = free_area_available(a); if (a->after) { - uint64_t need, space; + uint64_t need, space_end, new_end; need = partition_min_size_with_padding(a->after); assert(a->after->offset != UINT64_MAX); assert(a->after->current_size != UINT64_MAX); - space = round_up_size(a->after->offset + a->after->current_size, 4096) - a->after->offset + avail; - if (need >= space) - return 0; + /* Calculate where the free area ends, based on the offset of the partition preceding it */ + space_end = round_up_size(a->after->offset + a->after->current_size, 4096) + avail; - return space - need; + /* Calculate where the partition would end when we give it as much as it needs */ + new_end = round_up_size(a->after->offset + need, 4096); + + /* Calculate saturated difference of the two: that's how much we have free for other partitions */ + return LESS_BY(space_end, new_end); } return avail; From 15c597764424214617034165bbc62d90f7cf1ae5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 16:53:11 +0100 Subject: [PATCH 5/5] test: extend repart test suite to test for unaligned partitions --- test/units/testsuite-58.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/units/testsuite-58.sh b/test/units/testsuite-58.sh index 16cd8385fb..62d0a29cb5 100755 --- a/test/units/testsuite-58.sh +++ b/test/units/testsuite-58.sh @@ -84,6 +84,38 @@ cmp /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img rm /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img /tmp/testsuite-58.dump rm -r /tmp/testsuite-58-defs/ +# Third part: operate on an an image with unaligned partition, to see if that works. + +rm -f /var/tmp/testsuite-58.3.img /tmp/testsuite-58-3.dump +mkdir -p /tmp/testsuite-58.3-defs/ + +cat >/tmp/testsuite-58.3-defs/root.conf </testok exit 0