[moved to -fs]

In message <[EMAIL PROTECTED]>, 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 [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message

Reply via email to