Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Jan 2002 18:00:37 +1100 (EST)
From:      Bruce Evans <bde@zeta.org.au>
To:        Michael Reifenberger <root@nihil.plaut.de>
Cc:        FreeBSD-Current <current@FreeBSD.ORG>
Subject:   Re: panic during fdisk'ing a md(4) device
Message-ID:  <20020110175753.S11249-100000@gamplex.bde.org>
In-Reply-To: <20020109135621.H415-100000@nihil>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 9 Jan 2002, Michael Reifenberger wrote:

> On Wed, 9 Jan 2002, Bruce Evans wrote:
> ...
> > This seems to be just another null pointer panic caused by the dk macros
> > creating half-baked devices with null devswitches.  I sent the following
> > quick fixes for this to the devfs maintainer a couple of weeks ago.  They
> > also fix the non-creation of all minor devices on the disk when the first
> > one is opened.
> Applying your patch to a fresh -current tree leads to a page-fault in (swapper)
> after "mounting root from ..."
> (I had to apply the second Hunk of subr_disk by hand)

Hmm.  I used an old set of patches to avoid filtering out local changes
again.  Try the enclosed up to date patches.

> Do you know how to enable a dumpdev via the boot loader in order to
> get a dump?

Not really.  I wouldn't trust dumpdev early here since the bugs are in the
disk layer.

%%%
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	9 Jan 2002 10:34:30 -0000
@@ -79,6 +79,8 @@
 			    UID_ROOT, GID_OPERATOR, 0640, "%s%ds%d",
 				dp->d_devsw->d_name, u, s - BASE_SLICE + 1);
+#if 0
 			make_dev_alias(dev, "%s%ds%dc",
 			    dp->d_devsw->d_name, u, s - BASE_SLICE + 1);
+#endif
 		}
 		dev_depends(pdev, dev);
@@ -150,6 +152,8 @@
 			    UID_ROOT, GID_OPERATOR, 0640, "%s%ds%d",
 			    pdev->si_devsw->d_name, u, s - BASE_SLICE + 1);
+#if 0
 			make_dev_alias(*dev, "%s%ds%dc",
 			    pdev->si_devsw->d_name, u, s - BASE_SLICE + 1);
+#endif
 		} else {
 			*dev = make_dev(pdev->si_devsw, dkmakeminor(u, s, p),
@@ -301,5 +305,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 +353,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 +369,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 +404,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 +420,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.96
diff -u -2 -r1.96 subr_diskslice.c
--- kern/subr_diskslice.c	12 Sep 2001 08:37:45 -0000	1.96
+++ kern/subr_diskslice.c	9 Jan 2002 10:34:30 -0000
@@ -77,4 +77,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));
@@ -649,4 +650,5 @@
 	char	*msg;
 	u_char	mask;
+	char	*oldsname;
 	int	part;
 	char	partname[2];
@@ -728,11 +730,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
@@ -969,6 +989,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 devices 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
Index: sys/disklabel.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/disklabel.h,v
retrieving revision 1.63
diff -u -2 -r1.63 disklabel.h
--- sys/disklabel.h	4 Nov 2001 09:01:02 -0000	1.63
+++ sys/disklabel.h	9 Jan 2002 10:34:42 -0000
@@ -438,16 +438,4 @@
 				(((slice) << 16) | (((unit) & 0x1e0) << 16) | \
 				(((unit) & 0x1f) << 3) | (part))
-static __inline dev_t
-dkmodpart(dev_t dev, int part)
-{
-	return (makedev(major(dev), (minor(dev) & ~7) | part));
-}
-
-static __inline dev_t
-dkmodslice(dev_t dev, int slice)
-{
-	return (makedev(major(dev), (minor(dev) & ~0x1f0000) | (slice << 16)));
-}
-
 #define	dkpart(dev)		(minor(dev) & 7)
 #define	dkslice(dev)		((minor(dev) >> 16) & 0x1f)
@@ -478,4 +466,88 @@
 void	alpha_fix_srm_checksum __P((struct buf *bp));
 #endif
+
+#include <sys/conf.h>		/* XXX */
+#include <sys/diskslice.h>	/* XXX */
+
+/*
+ * 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 __inline 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 || newdev->si_flags & SI_NAMED)
+		return (newdev);
+	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)
+		return (make_dev(dev->si_devsw, mynor, dev->si_uid,
+		    dev->si_gid, dev->si_mode, "%s%s", sname, partname));
+	newdev = make_dev(dev->si_devsw, mynor, dev->si_uid,
+	    dev->si_gid, dev->si_mode, "%s", sname);
+	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 proof of prototype version.
+	 * Now there are problems getting the aliase created if the label
+	 * is discovered later.
+	 */
+	if (slicehint)
+		newdev_alias = NULL;
+	else
+		newdev_alias = make_dev_alias(newdev, "%s%s", sname, partname);
+#endif
+	return (slicehint ? newdev : newdev_alias);
+}
+
+static __inline dev_t
+dkmodpart(dev_t dev, int 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 = 0;
+		part = RAW_PART;
+	} else
+		slicehint = 0;
+	return (dkmodminor(dev, (minor(dev) & ~7) | part, slicehint));
+}
+
+static __inline dev_t
+dkmodslice(dev_t dev, int slice)
+{
+	/*
+	 * 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));
+}

 #endif /* _KERNEL */
%%%

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?20020110175753.S11249-100000>