From owner-freebsd-current Wed Jan 9 23: 0:17 2002 Delivered-To: freebsd-current@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 5FA7937B419 for ; Wed, 9 Jan 2002 23:00:00 -0800 (PST) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id RAA26254; Thu, 10 Jan 2002 17:59:50 +1100 Date: Thu, 10 Jan 2002 18:00:37 +1100 (EST) From: Bruce Evans X-X-Sender: To: Michael Reifenberger Cc: FreeBSD-Current Subject: Re: panic during fdisk'ing a md(4) device In-Reply-To: <20020109135621.H415-100000@nihil> Message-ID: <20020110175753.S11249-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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 /* XXX */ +#include /* 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