From owner-freebsd-fs Sun Jan 7 20: 4: 7 2001 Delivered-To: freebsd-fs@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 13C8D37B400 for ; Sun, 7 Jan 2001 20:03:49 -0800 (PST) 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 PAA25359; Mon, 8 Jan 2001 15:03:38 +1100 Date: Mon, 8 Jan 2001 15:04:30 +1100 (EST) From: Bruce Evans X-Sender: bde@besplex.bde.org To: Ian Dowse Cc: freebsd-fs@FreeBSD.ORG, Jaye Mathisen , Kirk McKusick Subject: Re: fsck problem on large vinum volume In-Reply-To: <200101071957.aa07352@salmon.maths.tcd.ie> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Sun, 7 Jan 2001, Ian Dowse wrote: > Here is a patch which should avoid the possibility of overflowing > the fs_csp[] array. The idea is that since all summary blocks are > stored in one contiguous malloc'd region, there is no need to > have a separate pointer to the start of each block within that > region. I noticed that the array was an obfuscation when I converted ext2fs to not have private caching for bitmap and summary blocks (except the malloc'd region is used to give a better form of private caching). (The changes have not been committed.) The corresponding array in ext2fs contained pointers to buffers. Buffers may be discontiguous so the array was really needed. > This is achieved by simplifying the 'fs_cs' macro from > > fs_csp[(indx) >> (fs)->fs_csshift][(indx) & ~(fs)->fs_csmask] > to > fs_csp[0][indx] > > so that only the start of the malloc'd region is needed, and can always > be placed in fs_csp[0] without the risk of overflow. Here is the corresponding macro for ext2fs: #define fs_cs(fs, cgindx) (fs->s_group_desc[cgindx]) I changed the array of pointers to a single pointer. Since fs_csp is in-core only, it too can be changed without affecting utilities. > Index: ffs/ffs_vfsops.c > =================================================================== > RCS file: /home/iedowse/CVS/src/sys/ufs/ffs/ffs_vfsops.c,v > retrieving revision 1.134 > diff -u -r1.134 ffs_vfsops.c > --- ffs/ffs_vfsops.c 2000/12/13 10:03:52 1.134 > +++ ffs/ffs_vfsops.c 2001/01/07 19:04:06 > @@ -365,7 +365,7 @@ > { > register struct vnode *vp, *nvp, *devvp; > struct inode *ip; > - struct csum *space; > + caddr_t space; caddr_t is a bogus type for almost everything. `space' should have type `void *', but we need to do pointer arithmetic with a 1-byte stride on it. The "opaque" type caddr_t works because it happens to be `char *'. I didn't fix this. > Index: ffs/fs.h > =================================================================== > RCS file: /home/iedowse/CVS/src/sys/ufs/ffs/fs.h,v > retrieving revision 1.16 > diff -u -r1.16 fs.h > --- ffs/fs.h 2000/07/04 04:55:48 1.16 > +++ ffs/fs.h 2001/01/07 18:55:44 > @@ -108,10 +108,10 @@ > /* > * The limit on the amount of summary information per file system > * is defined by MAXCSBUFS. It is currently parameterized for a > - * size of 128 bytes (2 million cylinder groups on machines with > - * 32-bit pointers, and 1 million on 64-bit machines). One pointer > - * is taken away to point to an array of cluster sizes that is > - * computed as cylinder groups are inspected. > + * size of 128 bytes. One pointer is taken away to point to an array > + * of cluster sizes that is computed as cylinder groups are inspected. > + * > + * Currently, the ffs code uses only the first entry. > */ I think we will never need more than 1 entry. The malloc'd region is quite small. (I only checked this for 1-4GB filesystems, but it was very small for these IIRC.) Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message