From owner-freebsd-current Fri Jan 18 4:28:10 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 DB6C137B419; Fri, 18 Jan 2002 04:27:54 -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 XAA07397; Fri, 18 Jan 2002 23:27:44 +1100 Date: Fri, 18 Jan 2002 23:29:03 +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: <20020117225217.O521-101000@nihil> Message-ID: <20020118232153.H792-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 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