Date: Fri, 18 Jan 2002 23:29:03 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Michael Reifenberger <root@nihil.plaut.de> Cc: FreeBSD-Current <current@FreeBSD.ORG>, <phk@FreeBSD.ORG> Subject: Re: panic during fdisk'ing a md(4) device Message-ID: <20020118232153.H792-100000@gamplex.bde.org> In-Reply-To: <20020117225217.O521-101000@nihil>
next in thread | previous in thread | raw e-mail | index | archive | help
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 majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020118232153.H792-100000>