Date: Sun, 23 Jun 2002 14:03:09 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Kirk McKusick <mckusick@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, <cvs-all@FreeBSD.org> Subject: Re: cvs commit: src/sys/ufs/ffs ffs_alloc.c Message-ID: <20020623134003.M10753-100000@gamplex.bde.org> In-Reply-To: <200206222124.g5MLOw243907@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020623134003.M10753-100000>