From owner-cvs-all Sat Jun 22 20:58:23 2002 Delivered-To: cvs-all@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 69FCD37B401; Sat, 22 Jun 2002 20:58:15 -0700 (PDT) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id NAA21843; Sun, 23 Jun 2002 13:58:07 +1000 Date: Sun, 23 Jun 2002 14:03:09 +1000 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Kirk McKusick Cc: cvs-committers@FreeBSD.org, Subject: Re: cvs commit: src/sys/ufs/ffs ffs_alloc.c In-Reply-To: <200206222124.g5MLOw243907@freefall.freebsd.org> Message-ID: <20020623134003.M10753-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sat, 22 Jun 2002, Kirk McKusick wrote: > mckusick 2002/06/22 14:24:58 PDT > > Modified files: > sys/ufs/ffs ffs_alloc.c > Log: > This patch fixes a problem whereby filesystems that ran > out of inodes in a cylinder group would fail to check for > free inodes in other cylinder groups. This bug was introduced > in the UFS2 code merge two days ago. > > An inode is allocated by calling ffs_valloc which calls > ffs_hashalloc to do the filesystem scan. Ffs_hashalloc > walks around the cylinder groups calling its passed allocator > (ffs_nodealloccg in this case) until the allocator returns a > non-zero result. The bug is that ffs_hashalloc expects the > passed allocator function to return a 64-bit ufs2_daddr_t. > When allocating inodes, it calls ffs_nodealloccg which was > returning a 32-bit ino_t. The ffs_hashalloc code checked > a 64-bit return value and usually found random non-zero bits in > the high 32-bits so decided that the allocation had succeeded > (in this case in the only cylinder group that it checked). > When the result was passed back to ffs_valloc it looked at > only the bottom 32-bits, saw zero and declared the system > out of inodes. But ffs_hashalloc had really only checked > one cylinder group. > > The fix is to change ffs_nodealloccg to return 64-bit results. This bug has a long history. I first noticed it when I added full prototypes to the kernel in 1995. Rev.1.20 broke a warning about it using a bogus cast. I didn't merge rev.1.20 until last year when warnings became errors. Then I replaced the warning by a comment: %%% Index: ffs_alloc.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_alloc.c,v retrieving revision 1.93 diff -u -2 -r1.93 ffs_alloc.c --- ffs_alloc.c 21 Jun 2002 06:18:03 -0000 1.93 +++ ffs_alloc.c 22 Jun 2002 18:17:06 -0000 @@ -855,6 +889,13 @@ fs->fs_contigdirs[cg]--; } + /* + * XXX bogusly cast ffs_nodealloccg to break the warning about the + * bug that ffs_nodealloccg is not suitable for use here. It + * returns ino_t, but alloc-functions must return ufs_daddr_t. + * This currently more or less works because ino_t is int32_t and + * ufs_daddr_t is u_int32_t. + */ ino = (ino_t)ffs_hashalloc(pip, cg, ipref, mode, - (allocfcn_t *)ffs_nodealloccg); + (allocfcn_t *)ffs_nodealloccg); if (ino == 0) goto noinodes; %%% Fixing this correctly wasn't possible until UFS2, since large ino_t's were not representable as ufs_daddr_t's. Returning ufs_daddr_t from ffs_nodealloccg() would have worked in practice on 2's complement machines. Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message