mkimg: Add per-partition alignment
AcceptedPublic
Actions

Authored by cperciva on Thu, Oct 8, 1:01 AM.

Details

Reviewers

manu
olivier
markj
imp

Summary

This adds a "%align" field to partition specifications, resulting in
the start of a partition being shifted to the next aligned position.
When both :offset and %align are used, a relative offset is treated as
a minimum value, while an unaligned absolute offset is an error.

MFC after: 1 week
Sponsored by: Amazon

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77826
Build 74709: arc lint + arc unit

Event Timeline

cperciva created this revision.Thu, Oct 8, 1:01 AM

Herald added a subscriber: imp. · View Herald TranscriptThu, Oct 8, 1:01 AM

cperciva requested review of this revision.Thu, Oct 8, 1:01 AM

Harbormaster completed remote builds in B77826: Diff 189009.Thu, Oct 8, 1:01 AM

cperciva added a child revision: D60451: mkimg.1: Document new %align mechanism.Thu, Oct 8, 1:01 AM

cperciva added reviewers: manu, olivier, markj.Thu, Oct 8, 1:04 AM

jlduran mentioned this in D60455: EC2: Align partitions to 1 MB boundaries.Thu, Oct 8, 3:41 AM

Shouldn't this also get a man page update?

markj added inline comments.Thu, Oct 8, 2:54 PM

usr.bin/mkimg/mkimg.c
520	It seems fine to have `bytealign % secsz != 0` so long as `bytealign` divides `secsz`, as you don't have to do any extra work in that case, i.e., `blkalign` is 1. Should we also verify that `bytealign % blksz != 0`?

In D60450#1387074, @markj wrote:

Shouldn't this also get a man page update?

Yes, that's https://reviews.freebsd.org/D60451 :-)

Any reason not to add -A alignment to set this globally?

This revision is now accepted and ready to land.Thu, Oct 8, 4:00 PM

cperciva added inline comments.Thu, Oct 8, 4:08 PM

usr.bin/mkimg/mkimg.c
520	I suppose we could accept e.g. `-p freebsd-swap/swapfs::1GB%1` but it's hard to imagine a circumstance when someone would do that deliberately. Should we also verify that bytealign % blksz != 0? Probably yes. And for that matter the handling of blkoffset seems wrong: blkoffset = (byteoffset + secsz - 1) / secsz; Presumably that should be blksz rather than secsz?

In D60450#1387239, @imp wrote:

Any reason not to add -A alignment to set this globally?

I was considering that, but as I wrote in https://reviews.freebsd.org/D60454 I'm not sure about whether aligning boot partitions is a good idea, so I figured it was better to leave it as a partition-by-partition option.

markj added inline comments.Thu, Oct 8, 4:14 PM

usr.bin/mkimg/mkimg.c
520	I suppose we could accept e.g. -p freebsd-swap/swapfs::1GB%1 but it's hard to imagine a circumstance when someone would do that deliberately. I was thinking of a case where someone wants a partition to be aligned to some power of two between 512 and 4096, two common sector sizes. It would be nice if you could explicitly specify the alignment in both cases, even though with secsz=4096 it would be redundant.

In D60450#1387264, @cperciva wrote:

In D60450#1387239, @imp wrote:

Any reason not to add -A alignment to set this globally?

I was considering that, but as I wrote in https://reviews.freebsd.org/D60454 I'm not sure about whether aligning boot partitions is a good idea, so I figured it was better to leave it as a partition-by-partition option.

Boot partitions are fine, with very very limited exceptions. ESPs and the x86 boot partitions certainly are fine to align like this. And it doesn't take a ton of unaligned writes to create heartburn for some FTL implementations.

Revision Contents
Changeset List

Path

Size

usr.bin/

mkimg/

mkimg.c

58 lines

Diff 189009

View Options

mkimg: Add per-partition alignmentAcceptedPublicActions