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>
