[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] docs: Update for stopping small mbr gap support
From: |
Daniel Kiper |
Subject: |
Re: [PATCH v2] docs: Update for stopping small mbr gap support |
Date: |
Fri, 6 Mar 2020 12:38:09 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Fri, Mar 06, 2020 at 11:38:49AM +0100, Javier Martinez Canillas wrote:
> Hello Michael,
>
> On 3/6/20 7:53 AM, Michael Chang wrote:
> > Further to the discussion about disabling btrfs zstd support for
> > i386-pc[1], this paragraph in manual about mbr gap size doesn't seem to
> > hold true any longer.
> >
> > "You must ensure that the first partition starts at least 31 KiB (63
> > sectors) from the start of the disk"
> >
> > As in many occasions we inevitablely have to provide core image with the
> > size that goes beyond 31 KiB. For instance, diskfilter and crypto
> > modules which are needed by root disk formatted with btrfs, lvm, mdadm
> > and so on would add quite a lot space to the image.
> >
> > So this misinformation would have people misguided and thought that it
> > is still fine to use small MBR gap utill some point of time the update
> > has grown the size too much that the grub-install can no longer embed
> > the image to the mbr gap. In this case changing the partition layout is
> > required but it is never easy to do so.
> >
> > The patch tries to correct the paragraph with a more practical size that
> > works for grub and also for modern computer systems in general.
> >
> > [1] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00025.html
> >
> > Signed-off-by: Michael Chang <address@hidden>
> > ---
> >
>
> Agreed with the patch.
>
> Reviewed-by: Javier Martinez Canillas <address@hidden>
Reviewed-by: Daniel Kiper <address@hidden>
...except some nitpicks which I fix before commiting the patch...
> > Changes since v2:
> > * Rework a paragraph in commit message and also some places in manual
> > to be more clear to read
> > * Correct some typos
> >
> > docs/grub.texi | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 83979af38..4614a2ee1 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -845,12 +845,20 @@ only be used if the @file{/boot} filesystem is on the
> > same disk that the
> > BIOS boots from, so that GRUB does not have to rely on guessing BIOS drive
> > numbers.
> >
> > -The GRUB development team generally recommends embedding GRUB before the
> > -first partition, unless you have special requirements. You must ensure
> > that
> > -the first partition starts at least 31 KiB (63 sectors) from the start of
> > -the disk; on modern disks, it is often a performance advantage to align
> > -partitions on larger boundaries anyway, so the first partition might start
> > 1
> > -MiB from the start of the disk.
> > +The GRUB development team generally recommends embedding GRUB before the
> > first
> > +partition, unless you have special requirements. You must ensure that the
> > first
> > +partition starts at least 1 MiB from the start of the disk; Additionally,
> > on
> > +modern disks it is often a performance advantage to align partitions on
> > larger
> > +boundaries and 1 MiB is the least common multiple of many used alignment
> > sizes.
> > +E.g. SSD, it became crucial to have the partition correctly aligned to
> > avoid
> > +excessive read-modify-write cycles and thus modern tools set to use 1 MiB
> > as a
> > +standard practice.
> > +
> > +In case of legacy systems that cannot boot if first partition is not on the
> > +cylinder boundary, the fallback blocklist install method should remain
> > working
> > +for them if the core image grows too much someday. Here we just can't
> > advertise
> > +that 31 KiB (63 sectors) is a sensible size any longer as that would pose
> > great
> > +constraint to include new features as time goes by.
>
> I think is also worth mentioning that most partition tools these days default
> to
> the optimal alignment for the disk. But could be done as a follow-up patch if
> you
> agree that adding this information would be useful.
This is not a must. However, if you think it makes sense please send a patch.
Daniel