From owner-freebsd-fs@FreeBSD.ORG Fri Aug 31 12:10:55 2007 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B4B0F16A419; Fri, 31 Aug 2007 12:10:55 +0000 (UTC) (envelope-from yar@comp.chem.msu.su) Received: from comp.chem.msu.su (comp.chem.msu.su [158.250.32.97]) by mx1.freebsd.org (Postfix) with ESMTP id 502BC13C4F5; Fri, 31 Aug 2007 12:10:53 +0000 (UTC) (envelope-from yar@comp.chem.msu.su) Received: from comp.chem.msu.su (localhost [127.0.0.1]) by comp.chem.msu.su (8.13.4/8.13.4) with ESMTP id l7VBeR0h091141; Fri, 31 Aug 2007 15:40:27 +0400 (MSD) (envelope-from yar@comp.chem.msu.su) Received: (from yar@localhost) by comp.chem.msu.su (8.13.4/8.13.4/Submit) id l7VBZK45091036; Fri, 31 Aug 2007 15:35:21 +0400 (MSD) (envelope-from yar) Date: Fri, 31 Aug 2007 15:35:19 +0400 From: Yar Tikhiy To: "Loren M. Lang" Message-ID: <20070831113519.GE85633@comp.chem.msu.su> References: <46D4F12D.3080107@north-winds.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46D4F12D.3080107@north-winds.org> User-Agent: Mutt/1.5.9i Cc: freebsd-fs , mckusick@FreeBSD.org Subject: Re: Bug in Newfs setting max-extend-size X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Aug 2007 12:10:55 -0000 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 . 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