Date: Sat, 14 Dec 2002 19:03:53 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Nate Lawson <nate@root.org> Cc: Julian Elischer <julian@elischer.org>, FreeBSD current users <current@FreeBSD.ORG>, <re@FreeBSD.ORG> Subject: Re: 5.0 showstopper Message-ID: <20021214183411.H5369-100000@gamplex.bde.org> In-Reply-To: <Pine.BSF.4.21.0212131710440.37808-100000@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 13 Dec 2002, Nate Lawson wrote:
> On Fri, 13 Dec 2002, Julian Elischer wrote:
> > On Fri, 13 Dec 2002, Peter Wemm wrote:
> > > Julian Elischer wrote:
> > > > looking at the code in src/sys/i386/i386/dump_machdep.c,
> > > >
> > > > we see:
> > > > 78 dumplo = di->mediaoffset + di->mediasize - Maxmem *
> > > > (off_t)PAGE_SIZE;
> > > > 79 dumplo -= sizeof kdh * 2;
> > > > 80 i = di->dumper(di->priv, &kdh, 0, dumplo, sizeof kdh);
> > > > 81 if (i)
> > > > 82 printf("\nDump failed writing header (%d)\n", i);
> > > > 83 dumplo += sizeof kdh;
> > > > 84 i = 0;
> > > >
> > > > It looks like the following test should go after line 77
> > > >
> > > > if (di->mediasize < ((Maxmem * (off_t)PAGE_SIZE) +
> > > > (sizeof kdh * 2) + (16*1024)) {
> > > > /* 16K is an arbitrary buffer
> > > > * in case the swap part is
> > > > * the first part
> > > > */
> > > > printf("\nDump failed. Partition too small.\n");
> > > > return;
> > > > }
> > >
> > > We used to force a 64K safety buffer for swapon, and I think we also did
> > > 64K for dump before the last MI dump code conversion. I'd feel a little
> > > happier if you used 64K here instead of 16K.
> >
> > I'm happy with 64k.. I just chose 16 arbitrarily because I remembered
> > that there was SOME safety value but I couldn't remember what it was..
>
> 1. Why 2 * kdh (the header size)? I thought there was just one.
2 * kdh is subtracted from dumplo for some reason. I don't know if the
second kdh is just supertitious or an obfuscated way of leaving more space
for metadata or necessary.
> 2. Excessive parens before compare
These are to obfuscate the code by making readers parse all the parentheses
only to discover that there is nothing special going on -- the expression
is just "a * b + c * d + e * f" obfuscated to "((a * b) + (c * d) + (e * f))".
> 3. Extra \n in printf
This is needed in some places (to terminate the dump status output)
the not here or in some of the other places that it is used.
> So how about:
> /*
> * Minimum dumpdev size is Maxmem, dump header,
> * and 64k safety margin.
> */
Wording could be improved.
> if (di->mediasize < Maxmem * (off_t)PAGE_SIZE + sizeof(kdh) +
> 64 * 1024) {
> printf("Dump failed. Partition too small.\n");
Wording could be improved. Most error messages including most of the ones
in this routine, are not terminated by a ".".
I tried to get brian to fix this back in October after he fixed the special
case of an empty dump device. The following points don't seem to have been
mentioned in this round:
- negative dumplo's are almost harmless because they cause negative physical
block numbers which the hardware rejects.
- the space needed for metadata is machine-dependent. RELENG_4 leaves
enough space for the label and all sectors before it. Since the i386
MBR is before the label, it gets protected too, even in the dangerously
dedicated case where the MBR is on the same % slice as the dump partition
(and the dump partition is at offset 0). The swapper leaves a fairly
arbitrary and much larger than normally necessary space for metadata
(64K IIRC). But dumping never agreed with swapping about this amount
(64K protects the label from swapping and LABELSECTOR magic protects
the label and much more from dumping).
- dumping is broken similarly on some other arches (ia64's at least).
- checking the bounds at the top level doesn't work in general because
the dump size is not really knwon there (the dump might be sparse, or
compressed...).
My fix for dumping problems was to turn of dumping when it was
second-systemed (actually third-systemed).
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021214183411.H5369-100000>