From owner-freebsd-hackers Sun Jan 7 11:59:15 2001 Delivered-To: freebsd-hackers@freebsd.org Received: from salmon.maths.tcd.ie (salmon.maths.tcd.ie [134.226.81.11]) by hub.freebsd.org (Postfix) with SMTP id 0935F37B400 for ; Sun, 7 Jan 2001 11:58:53 -0800 (PST) Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP id ; 7 Jan 2001 19:58:51 +0000 (GMT) X-Mmdf-To: iedowse Received: from salmon.maths.tcd.ie by maccullagh.maths.tcd.ie with SMTP id ; 7 Jan 2001 19:57:56 +0000 (GMT) Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP id ; 7 Jan 2001 19:57:56 +0000 (GMT) To: freebsd-fs@freebsd.org Cc: Jaye Mathisen , Kirk McKusick , iedowse@maths.tcd.ie Subject: Re: fsck problem on large vinum volume In-Reply-To: Your message of "Sun, 07 Jan 2001 15:07:54 GMT." <200101071507.aa79033@salmon.maths.tcd.ie> Date: Sun, 07 Jan 2001 19:57:55 +0000 From: Ian Dowse Message-ID: <200101071957.aa07352@salmon.maths.tcd.ie> Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG [moved to -fs] In message <200101071507.aa79033@salmon.maths.tcd.ie>, Ian Dowse writes: > >Jaye sent me a ktrace.out for the fsck that was failing. It appears >that the kernel had overshot the end of the superblock fs_csp[] array >in ffs_mountfs(), since the list of pointers there extended through >fs_maxcluster, fs_cpc, and fs_opostbl. This caused the mismatch between >the master and alternate superblocks. > >The filesystem parameters were 8k/1k, and the total number of cylinder >groups was 29782. fs_cssize was 29782*sizeof(struct csum) = 477184 >bytes. Hence 477184/8192 = ~59 entries were being used in fs_csp, >but fs_csp[] is only 31 entries long (15 on alpha). 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. 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. I have only tested this to the extent that the kernel compiles and runs, and only on -stable. Any comments or suggestions? Ian 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; struct buf *bp; struct fs *fs, *newfs; struct partinfo dpart; @@ -432,7 +432,7 @@ * Step 3: re-read summary information from disk. */ blks = howmany(fs->fs_cssize, fs->fs_fsize); - space = fs->fs_csp[0]; + space = (caddr_t)fs->fs_csp[0]; for (i = 0; i < blks; i += fs->fs_frag) { size = fs->fs_bsize; if (i + fs->fs_frag > blks) @@ -441,7 +441,8 @@ NOCRED, &bp); if (error) return (error); - bcopy(bp->b_data, fs->fs_csp[fragstoblks(fs, i)], (u_int)size); + bcopy(bp->b_data, space, (u_int)size); + space += size; brelse(bp); } /* @@ -513,7 +514,7 @@ register struct fs *fs; dev_t dev; struct partinfo dpart; - caddr_t base, space; + caddr_t space; int error, i, blks, size, ronly; int32_t *lp; struct ucred *cred; @@ -623,18 +624,18 @@ blks = howmany(size, fs->fs_fsize); if (fs->fs_contigsumsize > 0) size += fs->fs_ncg * sizeof(int32_t); - base = space = malloc((u_long)size, M_UFSMNT, M_WAITOK); + space = malloc((u_long)size, M_UFSMNT, M_WAITOK); + fs->fs_csp[0] = (struct csum *)space; for (i = 0; i < blks; i += fs->fs_frag) { size = fs->fs_bsize; if (i + fs->fs_frag > blks) size = (blks - i) * fs->fs_fsize; if ((error = bread(devvp, fsbtodb(fs, fs->fs_csaddr + i), size, cred, &bp)) != 0) { - free(base, M_UFSMNT); + free(fs->fs_csp[0], M_UFSMNT); goto out; } bcopy(bp->b_data, space, (u_int)size); - fs->fs_csp[fragstoblks(fs, i)] = (struct csum *)space; space += size; brelse(bp); bp = NULL; @@ -691,7 +692,7 @@ if (ronly == 0) { if ((fs->fs_flags & FS_DOSOFTDEP) && (error = softdep_mount(devvp, mp, fs, cred)) != 0) { - free(base, M_UFSMNT); + free(fs->fs_csp[0], M_UFSMNT); goto out; } if (fs->fs_snapinum[0] != 0) 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. */ #define MAXCSBUFS ((128 / sizeof(void *)) - 1) @@ -167,9 +167,6 @@ * from first cylinder group data blocks. These blocks have to be * read in from fs_csaddr (size fs_cssize) in addition to the * super block. - * - * N.B. sizeof(struct csum) must be a power of two in order for - * the ``fs_cs'' macro to work (see below). */ struct csum { int32_t cs_ndir; /* number of directories */ @@ -213,8 +210,8 @@ int32_t fs_fragshift; /* block to frag shift */ int32_t fs_fsbtodb; /* fsbtodb and dbtofsb shift constant */ int32_t fs_sbsize; /* actual size of super block */ - int32_t fs_csmask; /* csum block offset */ - int32_t fs_csshift; /* csum block number */ + int32_t fs_csmask; /* csum block offset (now unused) */ + int32_t fs_csshift; /* csum block number (now unused) */ int32_t fs_nindir; /* value of NINDIR */ int32_t fs_inopb; /* value of INOPB */ int32_t fs_nspf; /* value of NSPF */ @@ -328,11 +325,8 @@ /* * Convert cylinder group to base address of its global summary info. - * - * N.B. This macro assumes that sizeof(struct csum) is a power of two. */ -#define fs_cs(fs, indx) \ - fs_csp[(indx) >> (fs)->fs_csshift][(indx) & ~(fs)->fs_csmask] +#define fs_cs(fs, indx) fs_csp[0][indx] /* * Cylinder group block for a file system. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message