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>