Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Module problem on amd64



On Sun, 30 Nov 2008, Andrew Doran wrote:

Cool, thanks! I think there is MD code that plays with 'mountroot'. See:

http://nxr.homeunix.org/source/search?q=&defs=&refs=mountroot&path=&hist=

Yep, I've addressed the MD stuff in the latest revision - see attached diffs. The reference to 'mountroot' in next68k/stnd/boot/en.c is a false-positive; that one actually refers to a locally-defined function! All the other MD references to mountroot relate to NFS-booting on some hp* platforms.

TEST1 and TEST2 boot successfully to single user, and I'm able to do a
'ls' command from the single-user shell prompt.  However, both of these
panic due to the "end <= VM_MAX_KERNEL_ADDRESS" thing mentioned in an
earlier thread.

I will fix that soon.

Looking forward to it!   :)

KNF says 'strcmp(a, b) == 0'

Well, hopefully you meant 'strcmp(a,b) != 0' ??  I have fixed this.

+       static char my_vfs_name[NAME_MAX + 1];

Should be MNAMELEN from mount.h, I think.

Fixed in current diffs.

Can you make 'rootfstype' in ioconf.c (or wherever it is emitted) into an
array sized MNAMELEN and not a pointer? That way, we can get rid of
'mountroot' completely and always key on the name. So the code in
vfs_mountroot() would look like:

As previously mentioned,

On Sun, 30 Nov 2008, Paul Goyette wrote:

It's emitted in config's mkswap.c, and defined in sys/systm.h.

I've already completely removed the mountroot variable;  everything
already keys only on the name.  The only benefit of allocating the new
rootfstype as a fixed array is to avoid having the local 'my_vfs_name'
above.

But, if I define it as an array in swapnetbsd.c, it would also need to
be defined that way is sys/sys/systm.h, and then either systm.h would
need to #include mount.h, or all of its users would need to do that.

So, which is more desirable?

        1. #include <mount.h> in systm.h?
        2. #include <mount.h> in all users of systm.h?
        3. Have my_vfs_name statically allocated in kern_subr.c, retain
           rootfstype as a simple pointer, and make rootfstype point to
           it if needed?


Back to Andy's comments:

I have just noticed a bug in the existing code. In setroot():

http://nxr.homeunix.org/source/xref/sys/kern/kern_subr.c#928

If vops->vfs_mountroot == NULL, it should do vfs_delref(vops) before
continuing on.

Fixed in current diffs.

I added an additional test-case (with 'config ... type tmp') to test out the RB_ASKNAME path, and found an additional bug in my code. It has been addressed in the attached diffs.

Any additional comments/reviews/critiques greatly appreciated, as well as any test reports. I'd be especially interested in test reports from anyone using hpcmips, hpcsh, hp300, and hpcarm since these are the ones that had some MD references to mountroot.


----------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul%whooppee.com@localhost   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette%juniper.net@localhost |
----------------------------------------------------------------------
Index: sys/sys/systm.h
===================================================================
RCS file: /cvsroot/src/sys/sys/systm.h,v
retrieving revision 1.230
diff -u -p -r1.230 systm.h
--- sys/sys/systm.h     12 Nov 2008 14:29:31 -0000      1.230
+++ sys/sys/systm.h     30 Nov 2008 18:20:05 -0000
@@ -333,7 +333,7 @@ void        dopowerhooks(int);
  * these to be executed just before (*mountroot)() if the passed device is
  * selected as the root device.
  */
-extern int (*mountroot)(void);
+extern const char *rootfstype;
 void   *mountroothook_establish(void (*)(struct device *), struct device *);
 void   mountroothook_disestablish(void *);
 void   mountroothook_destroy(void);
Index: sys/kern/vfs_subr.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_subr.c,v
retrieving revision 1.357
diff -u -p -r1.357 vfs_subr.c
--- sys/kern/vfs_subr.c 24 Sep 2008 09:33:40 -0000      1.357
+++ sys/kern/vfs_subr.c 30 Nov 2008 18:20:06 -0000
@@ -2382,20 +2382,19 @@ vfs_mountroot(void)
        }
 
        /*
-        * If user specified a file system, use it.
-        */
-       if (mountroot != NULL) {
-               error = (*mountroot)();
-               goto done;
-       }
-
-       /*
         * Try each file system currently configured into the kernel.
         */
        mutex_enter(&vfs_list_lock);
        LIST_FOREACH(v, &vfs_list, vfs_list) {
                if (v->vfs_mountroot == NULL)
                        continue;
+               /*
+                * If user specified a file system type, use only
+                * that type
+                */
+               if (strcmp(rootfstype, "?") != 0 &&
+                   strcmp(rootfstype, v->vfs_name) != 0)
+                       continue;
 #ifdef DEBUG
                aprint_normal("mountroot: trying %s...\n", v->vfs_name);
 #endif
@@ -2420,7 +2419,6 @@ vfs_mountroot(void)
                error = EFTYPE;
        }
 
-done:
        if (error && device_class(root_device) == DV_DISK) {
                VOP_CLOSE(rootvp, FREAD, FSCRED);
                vrele(rootvp);
Index: sys/kern/kern_subr.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_subr.c,v
retrieving revision 1.196
diff -u -p -r1.196 kern_subr.c
--- sys/kern/kern_subr.c        27 Nov 2008 21:36:51 -0000      1.196
+++ sys/kern/kern_subr.c        30 Nov 2008 18:20:07 -0000
@@ -763,6 +763,7 @@ setroot(struct device *bootdv, int bootp
        struct ifnet *ifp;
        const char *deffsname;
        struct vfsops *vops;
+       static char my_vfs_name[MNAMELEN + 1];
 
 #ifdef TFTPROOT
        if (tftproot_dhcpboot(bootdv) != 0)
@@ -786,7 +787,7 @@ setroot(struct device *bootdv, int bootp
         * find a reasonable network interface for "rootspec".
         */
        vops = vfs_getopsbyname("nfs");
-       if (vops != NULL && vops->vfs_mountroot == mountroot &&
+       if (vops != NULL && strcmp(rootfstype, "nfs") == 0 &&
            rootspec == NULL &&
            (bootdv == NULL || device_class(bootdv) != DV_IFNET)) {
                IFNET_FOREACH(ifp) {
@@ -897,21 +898,23 @@ setroot(struct device *bootdv, int bootp
                for (vops = LIST_FIRST(&vfs_list); vops != NULL;
                     vops = LIST_NEXT(vops, vfs_list)) {
                        if (vops->vfs_mountroot != NULL &&
-                           vops->vfs_mountroot == mountroot)
+                           strcmp(rootfstype, vops->vfs_name) == 0)
                        break;
                }
 
-               if (vops == NULL) {
-                       mountroot = NULL;
+               if (vops == NULL)
                        deffsname = "generic";
-               } else
+               else
                        deffsname = vops->vfs_name;
 
                for (;;) {
                        printf("file system (default %s): ", deffsname);
                        len = cngetsn(buf, sizeof(buf));
-                       if (len == 0)
+                       if (len == 0) {
+                               if (strcmp(deffsname, "generic") == 0)
+                                       rootfstype = "?";
                                break;
+                       }
                        if (len == 4 && strcmp(buf, "halt") == 0)
                                cpu_reboot(RB_HALT, NULL);
                        else if (len == 6 && strcmp(buf, "reboot") == 0)
@@ -922,7 +925,7 @@ setroot(struct device *bootdv, int bootp
                        }
 #endif
                        else if (len == 7 && strcmp(buf, "generic") == 0) {
-                               mountroot = NULL;
+                               rootfstype = "?";
                                break;
                        }
                        vops = vfs_getopsbyname(buf);
@@ -934,12 +937,20 @@ setroot(struct device *bootdv, int bootp
                                        if (vops->vfs_mountroot != NULL)
                                                printf(" %s", vops->vfs_name);
                                }
+                               if (vops != NULL)
+                                       vfs_delref(vops);
 #if defined(DDB)
                                printf(" ddb");
 #endif
                                printf(" halt reboot\n");
                        } else {
-                               mountroot = vops->vfs_mountroot;
+                               /*
+                                * Copy vfs_name locally since *vops might
+                                * be deallocated.
+                                */
+                               strncpy(my_vfs_name, vops->vfs_name,
+                                       sizeof(my_vfs_name));
+                               rootfstype = my_vfs_name;
                                vfs_delref(vops);
                                break;
                        }
Index: sys/ufs/mfs/mfs_vfsops.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/mfs/mfs_vfsops.c,v
retrieving revision 1.99
diff -u -p -r1.99 mfs_vfsops.c
--- sys/ufs/mfs/mfs_vfsops.c    13 Nov 2008 11:10:41 -0000      1.99
+++ sys/ufs/mfs/mfs_vfsops.c    30 Nov 2008 18:20:08 -0000
@@ -253,7 +253,7 @@ mfs_initminiroot(void *base)
        if (fs->fs_magic != FS_UFS1_MAGIC || fs->fs_bsize > MAXBSIZE ||
            fs->fs_bsize < sizeof(struct fs))
                return (0);
-       mountroot = mfs_mountroot;
+       rootfstype = "mfs";
        mfs_rootbase = base;
        mfs_rootsize = fs->fs_fsize * fs->fs_size;
        rootdev = makedev(255, mfs_minor);
Index: sys/rump/librump/rumpvfs/vfsops_stub.c
===================================================================
RCS file: /cvsroot/src/sys/rump/librump/rumpvfs/vfsops_stub.c,v
retrieving revision 1.1
diff -u -p -r1.1 vfsops_stub.c
--- sys/rump/librump/rumpvfs/vfsops_stub.c      19 Nov 2008 14:10:49 -0000      
1.1
+++ sys/rump/librump/rumpvfs/vfsops_stub.c      30 Nov 2008 18:20:08 -0000
@@ -37,7 +37,7 @@
 #include <miscfs/fifofs/fifo.h>
 #include <miscfs/syncfs/syncfs.h>
 
-int (*mountroot)(void);
+const char *rootfstype;
 
 #define VFSSTUB(name)                                                  \
     int name(void *arg) {panic("%s: unimplemented vfs stub", __func__);}
Index: sys/arch/hpcmips/hpcmips/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hpcmips/hpcmips/machdep.c,v
retrieving revision 1.98
diff -u -p -r1.98 machdep.c
--- sys/arch/hpcmips/hpcmips/machdep.c  12 Nov 2008 12:36:01 -0000      1.98
+++ sys/arch/hpcmips/hpcmips/machdep.c  30 Nov 2008 18:20:09 -0000
@@ -418,7 +418,7 @@ mach_init(int argc, char *argv[], struct
                                /* boot device: -b=sd0 etc. */
 #ifdef NFS
                                if (strcmp(cp+2, "nfs") == 0)
-                                       mountroot = nfs_mountroot;
+                                       rootfstype = "nfs";
                                else
                                        makebootdev(cp+2);
 #else /* NFS */
Index: sys/arch/hpcsh/hpcsh/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hpcsh/hpcsh/machdep.c,v
retrieving revision 1.64
diff -u -p -r1.64 machdep.c
--- sys/arch/hpcsh/hpcsh/machdep.c      17 Nov 2008 02:05:13 -0000      1.64
+++ sys/arch/hpcsh/hpcsh/machdep.c      30 Nov 2008 18:20:10 -0000
@@ -243,7 +243,7 @@ machine_startup(int argc, char *argv[], 
                        p = cp + 2;
 #ifdef NFS
                        if (strcmp(p, "nfs") == 0)
-                               mountroot = nfs_mountroot;
+                               rootfstype = "nfs";
                        else
                                makebootdev(p);
 #else /* NFS */
Index: sys/arch/hp300/hp300/autoconf.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hp300/hp300/autoconf.c,v
retrieving revision 1.91
diff -u -p -r1.91 autoconf.c
--- sys/arch/hp300/hp300/autoconf.c     22 Jun 2008 16:29:36 -0000      1.91
+++ sys/arch/hp300/hp300/autoconf.c     30 Nov 2008 18:20:11 -0000
@@ -381,7 +381,8 @@ cpu_rootconf(void)
         */
        if (rootspec == NULL) {
                vops = vfs_getopsbyname("nfs");
-               if (vops != NULL && vops->vfs_mountroot == mountroot) {
+               if (vops != NULL && vops->vfs_mountroot != NULL &&
+                   strcmp(rootfstype, "nfs") == 0) {
                        for (dd = LIST_FIRST(&dev_data_list);
                            dd != NULL; dd = LIST_NEXT(dd, dd_list)) {
                                if (device_class(dd->dd_dev) == DV_IFNET) {
@@ -395,6 +396,8 @@ cpu_rootconf(void)
                                dv = NULL;
                        }
                }
+               if (vops != NULL)
+                       vfs_delref(vops);
        }
 
        /*
Index: sys/arch/hpcarm/hpcarm/hpc_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/hpcarm/hpcarm/hpc_machdep.c,v
retrieving revision 1.88
diff -u -p -r1.88 hpc_machdep.c
--- sys/arch/hpcarm/hpcarm/hpc_machdep.c        12 Nov 2008 12:36:01 -0000      
1.88
+++ sys/arch/hpcarm/hpcarm/hpc_machdep.c        30 Nov 2008 18:20:12 -0000
@@ -364,7 +364,7 @@ initarm(int argc, char **argv, struct bo
                        cp = cp + 2;
 #ifdef NFS
                        if (strcmp(cp, "nfs") == 0)
-                               mountroot = nfs_mountroot;
+                               rootfstype = "nfs";
                        else
                                strncpy(boot_file, cp, sizeof(boot_file));
 #else /* !NFS */
Index: usr.bin/config/mkswap.c
===================================================================
RCS file: /cvsroot/src/usr.bin/config/mkswap.c,v
retrieving revision 1.4
diff -u -p -r1.4 mkswap.c
--- usr.bin/config/mkswap.c     12 Dec 2007 00:03:34 -0000      1.4
+++ usr.bin/config/mkswap.c     30 Nov 2008 18:20:12 -0000
@@ -133,14 +133,8 @@ mkoneswap(struct config *cf)
        /*
         * Emit the root file system.
         */
-       if (cf->cf_fstype == NULL)
-               strlcpy(specinfo, "NULL", sizeof(specinfo));
-       else {
-               snprintf(specinfo, sizeof(specinfo), "%s_mountroot",
-                   cf->cf_fstype);
-               fprintf(fp, "int %s(void);\n", specinfo);
-       }
-       fprintf(fp, "int (*mountroot)(void) = %s;\n", specinfo);
+       fprintf(fp, "const char *rootfstype = \"%s\";\n",
+               cf->cf_fstype ? cf->cf_fstype : "?");
 
        fflush(fp);
        if (ferror(fp))


Home | Main Index | Thread Index | Old Index