Date: Mon, 8 Jan 2001 15:04:30 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Ian Dowse <iedowse@maths.tcd.ie> Cc: freebsd-fs@FreeBSD.ORG, Jaye Mathisen <mrcpu@internetcds.com>, Kirk McKusick <mckusick@mckusick.com> Subject: Re: fsck problem on large vinum volume Message-ID: <Pine.BSF.4.21.0101081353140.21594-100000@besplex.bde.org> In-Reply-To: <200101071957.aa07352@salmon.maths.tcd.ie>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0101081353140.21594-100000>