Date: Sun, 07 Jan 2001 19:57:55 +0000 From: Ian Dowse <iedowse@maths.tcd.ie> To: freebsd-fs@freebsd.org Cc: Jaye Mathisen <mrcpu@internetcds.com>, Kirk McKusick <mckusick@mckusick.com>, iedowse@maths.tcd.ie Subject: Re: fsck problem on large vinum volume Message-ID: <200101071957.aa07352@salmon.maths.tcd.ie> In-Reply-To: Your message of "Sun, 07 Jan 2001 15:07:54 GMT." <200101071507.aa79033@salmon.maths.tcd.ie>
next in thread | previous in thread | raw e-mail | index | archive | help
[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-fs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi? <200101071957.aa07352>