Date: Fri, 31 Aug 2007 15:35:19 +0400 From: Yar Tikhiy <yar@comp.chem.msu.su> To: "Loren M. Lang" <lorenl@north-winds.org> Cc: freebsd-fs <freebsd-fs@FreeBSD.org>, mckusick@FreeBSD.org Subject: Re: Bug in Newfs setting max-extend-size Message-ID: <20070831113519.GE85633@comp.chem.msu.su> In-Reply-To: <46D4F12D.3080107@north-winds.org> References: <46D4F12D.3080107@north-winds.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Aug 28, 2007 at 09:08:13PM -0700, Loren M. Lang wrote:
> I was reading through the newfs source code for FreeBSD 6.1-RELEASE and
> noticed some oddities between the man page for newfs and it's source
> related to max-extent-size (-d). Newfs claims that the default is 16
> times the filesystem blocksize. ("... It is presently limited to its
> default value which is 16 times the file system blocksize.") However,
> in the source code, if maxbsize is not specified, it is assigned bsize,
> not 16*bsize.
> newfs.c:
>
> if (maxbsize == 0)
> maxbsize = bsize;
As I can judge from the fact that the fs_maxbsize field isn't really
used by the kernel UFS code (except for one unrelated place dealing
with UFS1 superblock update), extent allocation hasn't been implemented
yet. In his paper on UFS2, Kirk said it would be future work. See
<http://www.usenix.org/events/bsdcon03/tech/mckusick.html>.
When it's actually there, its maximum value will be
FS_MAXCONTIG * bsize.
I don't think that the paragraph in the manpage should be considered
incorrect -- note the word "may" in it. :-)
> Also, in mkfs.c, mkfs() does some sanity checks on maxbsize, but in the
> second if statement on line 211, it checks sblock.fs_maxbsize, not
> maxbsize, but as far as I can tell, sblock.fs_maxbsize is not yet
> initialized.
> mkfs.c:
>
> if (maxbsize < bsize || !POWEROF2(maxbsize)) {
> sblock.fs_maxbsize = sblock.fs_bsize;
> printf("Extent size set to %d\n", sblock.fs_maxbsize);
> } else if (sblock.fs_maxbsize > FS_MAXCONTIG * sblock.fs_bsize) {
> sblock.fs_maxbsize = FS_MAXCONTIG * sblock.fs_bsize;
> printf("Extent size reduced to %d\n", sblock.fs_maxbsize);
> } else {
> sblock.fs_maxbsize = maxbsize;
> }
>
> Unless I am misunderstanding something, the else if() should read:
>
> } else if (maxbsize > FS_MAXCONTIG * sblock.fs_bsize) {
>
> This appears to be the same in FreeBSD-CURRENT as well.
This code indeed looks buggy. Besides using uninitialzed
sblock.fs_maxbsize, it uses bsize, which is a user-supplied
approximation, not a final value from the superblock. I think it
should read as follows to be correct:
if (maxbsize < sblock.fs_bsize || !POWEROF2(maxbsize)) {
^^^^^^^^^^^^^^^
sblock.fs_maxbsize = sblock.fs_bsize;
printf("Extent size set to %d\n", sblock.fs_maxbsize);
} else if (maxbsize > FS_MAXCONTIG * sblock.fs_bsize) {
^^^^^^^^
sblock.fs_maxbsize = FS_MAXCONTIG * sblock.fs_bsize;
printf("Extent size reduced to %d\n", sblock.fs_maxbsize);
} else {
sblock.fs_maxbsize = maxbsize;
}
--
Yar
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070831113519.GE85633>