On Thu, 17 Jan 2002, Michael Reifenberger wrote:

> On Thu, 17 Jan 2002, Bruce Evans wrote:
> ...
> > Oops.  There should be no alias for md10c.  Try this version.  It fixes
> > the "may want an alias case" in dkmodminor() and moves all the dk inlines
> > to subr_diskslice.c.
> This patch works better.
> This brings us back to the original problem :-)
> Now it doesn't panic during fsck immediately, it panics after unconfiguring
> the md(4) device (which leaves the /dev/md11* devices in place instead removing
> them) reconfiguring and then doing a new `disklabel ...` which tries to do
> a (still existing) new 'make_dev("md11s1")'....
> See the attached backroot.sh.

It somehow worked here, but after fixing the non-removal of the
/dev/md11*, I got back what is probably the original panic too.  At
least with my fixed changes, the panic is caused by not clearing
pointers in destroy_dev().  The pointers come back in make_dev() and
bite you a little later.  My orginal fixes helped by not doing all the
necessary calls to destroy_dev().

devfs still leaks 2 DEVFS allocations and 6 devbuf allocations for every
iteration of the script.

%%%
Index: kern/kern_conf.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_conf.c,v
retrieving revision 1.104
diff -u -2 -r1.104 kern_conf.c
--- kern/kern_conf.c    9 Jan 2002 04:58:49 -0000       1.104
+++ kern/kern_conf.c    18 Jan 2002 12:19:35 -0000
@@ -384,4 +384,5 @@
        dev->si_drv2 = 0;
        dev->si_devsw = 0;
+       bzero(&dev->__si_u, sizeof(dev->__si_u));       /* XXX */
        dev->si_flags &= ~SI_NAMED;
        dev->si_flags &= ~SI_ALIAS;
Index: kern/subr_disk.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_disk.c,v
retrieving revision 1.50
diff -u -2 -r1.50 subr_disk.c
--- kern/subr_disk.c    4 Nov 2001 11:56:22 -0000       1.50
+++ kern/subr_disk.c    14 Jan 2002 11:42:38 -0000
@@ -301,5 +301,5 @@

        error = 0;
-       pdev = dkmodpart(dkmodslice(dev, WHOLE_DISK_SLICE), RAW_PART);
+       pdev = dkmodslice(dkmodpart(dev, -RAW_PART), WHOLE_DISK_SLICE);

        dp = pdev->si_disk;
@@ -349,5 +349,5 @@

        error = 0;
-       pdev = dkmodpart(dkmodslice(dev, WHOLE_DISK_SLICE), RAW_PART);
+       pdev = dkmodslice(dkmodpart(dev, -RAW_PART), WHOLE_DISK_SLICE);
        dp = pdev->si_disk;
        if (!dp)
@@ -365,5 +365,5 @@
        struct disk *dp;

-       pdev = dkmodpart(dkmodslice(bp->bio_dev, WHOLE_DISK_SLICE), RAW_PART);
+       pdev = dkmodslice(dkmodpart(bp->bio_dev, -RAW_PART), WHOLE_DISK_SLICE);
        dp = pdev->si_disk;
        bp->bio_resid = bp->bio_bcount;
@@ -400,5 +400,5 @@
        dev_t pdev;

-       pdev = dkmodpart(dkmodslice(dev, WHOLE_DISK_SLICE), RAW_PART);
+       pdev = dkmodslice(dkmodpart(dev, -RAW_PART), WHOLE_DISK_SLICE);
        dp = pdev->si_disk;
        if (!dp)
@@ -416,5 +416,5 @@
        dev_t pdev;

-       pdev = dkmodpart(dkmodslice(dev, WHOLE_DISK_SLICE), RAW_PART);
+       pdev = dkmodslice(dkmodpart(dev, -RAW_PART), WHOLE_DISK_SLICE);
        dp = pdev->si_disk;
        if (!dp)
Index: kern/subr_diskmbr.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_diskmbr.c,v
retrieving revision 1.54
diff -u -2 -r1.54 subr_diskmbr.c
--- kern/subr_diskmbr.c 11 Dec 2001 05:35:43 -0000      1.54
+++ kern/subr_diskmbr.c 9 Jan 2002 10:34:30 -0000
@@ -209,5 +209,5 @@
        /* Read master boot record. */
        bp = geteblk((int)lp->d_secsize);
-       bp->b_dev = dkmodpart(dkmodslice(dev, WHOLE_DISK_SLICE), RAW_PART);
+       bp->b_dev = dkmodslice(dkmodpart(dev, -RAW_PART), WHOLE_DISK_SLICE);
        bp->b_blkno = mbr_offset;
        bp->b_bcount = lp->d_secsize;
Index: kern/subr_diskslice.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/subr_diskslice.c,v
retrieving revision 1.97
diff -u -2 -r1.97 subr_diskslice.c
--- kern/subr_diskslice.c       17 Jan 2002 18:33:18 -0000      1.97
+++ kern/subr_diskslice.c       18 Jan 2002 10:02:09 -0000
@@ -68,4 +68,5 @@

 static struct disklabel *clone_label __P((struct disklabel *lp));
+static dev_t dkmodminor __P((dev_t dev, int mynor, int slicehint));
 static void dsiodone __P((struct bio *bp));
 static char *fixlabel __P((char *sname, struct diskslice *sp,
@@ -77,4 +78,5 @@
                              struct disklabel *lp));
 static void set_ds_labeldevs __P((dev_t dev, struct diskslices *ssp));
+static void set_ds_labeldevs_unaliased __P((dev_t dev, struct diskslices *ssp));
 static void set_ds_wlabel __P((struct diskslices *ssp, int slice,
                               int wlabel));
@@ -122,8 +124,89 @@
 }

+/*
+ * XXX should be able to share more code between disk_dev_synth(),
+ * disk_clone() and here.
+ * XXX using dsname() only slightly insulates us from complications.
+ */
+static dev_t
+dkmodminor(dev_t dev, int mynor, int slicehint)
+{
+       dev_t newdev, newdev_alias;
+       const char *sname;
+       char partname[2];
+
+       newdev = makedev(major(dev), mynor);
+       if ((dev->si_flags & SI_NAMED) == 0)
+               return (newdev);        /* XXX should panic. */
+       if (newdev->si_flags & SI_NAMED) {
+               /* We have found a device, but may want an alias. */
+               if (dkslice(newdev) == WHOLE_DISK_SLICE ||
+                   dkslice(newdev) == COMPATIBILITY_SLICE ||
+                   dkpart(newdev) != RAW_PART || slicehint)
+                       return (newdev);
+
+               /* We do want an alias.  There can be only one.  XXX. */
+               newdev_alias = LIST_FIRST(&newdev->si_children);
+               if (newdev_alias != NULL)
+                       return (newdev_alias);
+               sname = dsname(dev, dkunit(newdev), dkslice(newdev),
+                   dkpart(newdev), partname);
+               return (make_dev_alias(newdev, "%s%s", sname, partname));
+       }
+       sname = dsname(dev, dkunit(newdev), dkslice(newdev), dkpart(newdev),
+           partname);
+       if (dkslice(newdev) == WHOLE_DISK_SLICE && dkpart(newdev) != RAW_PART) {
+               printf("bad disk name, sname = '%s', partname = '%s'\n",
+                   sname, partname);
+               Debugger("dkmod");
+       }
+       if (dkslice(newdev) == COMPATIBILITY_SLICE ||
+           dkpart(newdev) != RAW_PART) {
+               newdev = make_dev(dev->si_devsw, mynor, dev->si_uid,
+                   dev->si_gid, dev->si_mode, "%s%s", sname, partname);
+               dev_depends(dev, newdev);
+               return (newdev);
+       }
+       newdev = make_dev(dev->si_devsw, mynor, dev->si_uid,
+           dev->si_gid, dev->si_mode, "%s", sname);
+       dev_depends(dev, newdev);
+       if (dkslice(newdev) == WHOLE_DISK_SLICE)
+               return (newdev);
+#if 0
+       newdev_alias = make_dev_alias(newdev, "%s%s", sname, partname);
+#else
+       /*
+        * Don't blindly create the alias. since it is bogus if the slice
+        * is unlabeled.  Passing another hint to tell use when to do this
+        * would be too messy even for this prototype version.  Now there
+        * are problems getting the alias created if the label is discovered
+        * later (these are fixed here but not in subr_disk.c).
+        */
+       if (slicehint)
+               newdev_alias = NULL;
+       else
+               newdev_alias = make_dev_alias(newdev, "%s%s", sname, partname);
+#endif
+       return (slicehint ? newdev : newdev_alias);
+}
+
 dev_t
 dkmodpart(dev_t dev, int part)
 {
-       return (makedev(major(dev), (minor(dev) & ~7) | part));
+       int slicehint;
+
+       /*
+        * XXX temporary hack: callers pass part == -RAW_PART instead of
+        * part == RAW_PART as a hint that they want a device whose name
+        * doesn't contain the partition letter for RAW_PART, if possible.
+        * This is possible unless the slice is COMPATIBILITY_SLICE.  This
+        * is non-optional if the slice is WHOLE_DISK_SLICE.
+        */
+       if (part == -RAW_PART) {
+               slicehint = 1;
+               part = RAW_PART;
+       } else
+               slicehint = 0;
+       return (dkmodminor(dev, (minor(dev) & ~7) | part, slicehint));
 }

@@ -131,5 +214,12 @@
 dkmodslice(dev_t dev, int slice)
 {
-       return (makedev(major(dev), (minor(dev) & ~0x1f0000) | (slice << 16)));
+       /*
+        * Here we hint that we don't want a partition letter unless we
+        * don't already have one, our partition is RAW_PART, and our slice
+        * is not COMPATIBILITY_SLICE.  These cases are distinguished by
+        * SI_ALIAS being set.  The hint is not used in other cases.
+        */
+       return (dkmodminor(dev, (minor(dev) & ~0x1f0000) | (slice << 16),
+           (dev->si_flags & SI_ALIAS) == 0));
 }

@@ -667,4 +757,5 @@
        char    *msg;
        u_char  mask;
+       char    *oldsname;
        int     part;
        char    partname[2];
@@ -746,11 +837,29 @@
                    )
                        continue;
-               dev1 = dkmodslice(dkmodpart(dev, RAW_PART), slice);
-#if 0
-               sname = dsname(dev, unit, slice, RAW_PART, partname);
-#else
-               *partname='\0';
-               sname = dev1->si_name;
-#endif
+               dev1 = dkmodslice(dkmodpart(dev, -RAW_PART), slice);
+               if (dev1->si_devsw == NULL) {
+                       Debugger("dsopen: no devsw (can't happen)");
+                       dev1->si_devsw = dev->si_devsw;
+               }
+               /*
+                * XXX we want a device name without any partition letter
+                * in it for use in error messages.  dev1->si_name doesn't
+                * give this for the compatibility slice since there is no
+                * alias for the raw partiton on that slice.
+                *
+                * XXX dsname() is only used for the regression check;
+                * partname is only used to throw away the partition name
+                * in the regression check.
+                */
+               if (slice == COMPATIBILITY_SLICE)
+                       sname = dkmodslice(dkmodpart(dev, -RAW_PART),
+                           WHOLE_DISK_SLICE)->si_name;
+               else
+                       sname = dev1->si_name;
+               oldsname = dsname(dev, unit, slice, RAW_PART, partname);
+               if (strcmp(sname, oldsname) != 0)
+                       printf(
+               "dsopen: dsname = '%s', partname = '%s', sname = '%s'\n",
+                           oldsname, partname, sname);
                /*
                 * XXX this should probably only be done for the need_init
@@ -987,6 +1096,55 @@
        struct diskslices *ssp;
 {
+       int     slice;
+
+       set_ds_labeldevs_unaliased(dev, ssp);
+       if (ssp->dss_first_bsd_slice == COMPATIBILITY_SLICE)
+               return;
+       slice = dkslice(dev);
+       if (slice == COMPATIBILITY_SLICE)
+               set_ds_labeldevs_unaliased(
+                   dkmodslice(dev, ssp->dss_first_bsd_slice), ssp);
+       else if (slice == ssp->dss_first_bsd_slice)
+               set_ds_labeldevs_unaliased(
+                   dkmodslice(dev, COMPATIBILITY_SLICE), ssp);
 }

+static void
+set_ds_labeldevs_unaliased(dev, ssp)
+       dev_t   dev;
+       struct diskslices *ssp;
+{
+       struct disklabel *lp;
+       int     part;
+       struct partition *pp;
+       int     slice;
+       struct diskslice *sp;
+
+       slice = dkslice(dev);
+       sp = &ssp->dss_slices[slice];
+       if (sp->ds_size == 0)
+               return;
+       lp = sp->ds_label;
+       for (part = 0; part < lp->d_npartitions; part++) {
+               pp = &lp->d_partitions[part];
+               if (pp->p_size == 0)
+                       continue;
+               /*
+                * Just dkmod'ing to a partition creates all the necessary
+                * device entries for it.  This is a bit weird, but it
+                * corresponds to userland stat'ing of nonexistent devfs
+                * directory entries creating them, and at least we avoid
+                * creating entries for nonexistent empty devices here.
+                *
+                * XXX userland can even exploit bugs to create invalid
+                * devices, e.g., ones with slice numbers larger than the
+                * max.  Such slice numbers leak into the unit number
+                * or so-called "spare" bitfields.
+                */
+               if (dev->si_flags & SI_ALIAS)
+                       Debugger("unexpeced dk alias");
+               (void)dkmodpart(dev, part);
+       }
+}

 static void
%%%

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to