Re: unmount cgd panic

[David Young <dyoung%pobox.com@localhost>]

On Wed, Jan 20, 2010 at 04:55:51PM +0000, Patrick Welche wrote:
> On Wed, Jan 20, 2010 at 09:30:44AM -0600, David Young wrote:
> > On Wed, Jan 20, 2010 at 02:37:19PM +0000, Patrick Welche wrote:
> > > Everytime I shutdown a NetBSD-current/i386 system with a cgd partition,
> > > I get the following panic - anyone recognise a "recent" change (say last
> > > 3 weeks) that may cause this?
> > 
> > What does the system log between your typing 'shutdown' and this panic?
> 
> Odd: I just did a shutdown and didn't get a panic (first time), then tried
> again, and got the panic.
> 
> Log says syncing disks, and all filesystems *except* cgd0a or its associated
> wd0h, and root (/),  get unmounted, then pcmcia0, cardbus0, cardslot0, cbb0,
> audio0, sysbeep0, midi0, audio1, md0 get detached. Then panic with the same
> trace as before. (cgdclose)

I see the problem.  Try this patch.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933

Index: sys/dev/cgd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/cgd.c,v
retrieving revision 1.66
diff -p -u -u -p -r1.66 cgd.c
--- sys/dev/cgd.c       12 Jan 2010 23:49:34 -0000      1.66
+++ sys/dev/cgd.c       20 Jan 2010 17:28:28 -0000
@@ -193,23 +193,21 @@ cgd_attach(device_t parent, device_t sel
 static int
 cgd_detach(device_t self, int flags)
 {
-       int     ret = 0;
-       int     pmask = 1 << RAW_PART;
+       int ret;
+       const int pmask = 1 << RAW_PART;
        struct cgd_softc *sc = device_private(self);
-       struct dk_softc *dksc;
+       struct dk_softc *dksc = &sc->sc_dksc;
 
-       dksc = &sc->sc_dksc;
-       if ((dksc->sc_flags & DKF_INITED) != 0)
-       {
-               if (DK_BUSY(&sc->sc_dksc, pmask))
-                       ret = EBUSY;
-               else
-                       ret = cgd_ioctl_clr(sc, curlwp);
-       }
+       if (DK_BUSY(dksc, pmask))
+               return EBUSY;
 
-       disk_destroy(&sc->sc_dksc.sc_dkdev);
+       if ((dksc->sc_flags & DKF_INITED) != 0 &&
+           (ret = cgd_ioctl_clr(sc, curlwp)) != 0)
+               return ret;
 
-       return ret;
+       disk_destroy(&dksc->sc_dkdev);
+
+       return 0;
 }
 
 void
@@ -498,7 +496,6 @@ cgdioctl(dev_t dev, u_long cmd, void *da
        struct  cgd_softc *cs;
        struct  dk_softc *dksc;
        struct  disk *dk;
-       int     ret;
        int     part = DISKPART(dev);
        int     pmask = 1 << part;
 
@@ -517,18 +514,12 @@ cgdioctl(dev_t dev, u_long cmd, void *da
        switch (cmd) {
        case CGDIOCSET:
                if (dksc->sc_flags & DKF_INITED)
-                       ret = EBUSY;
-               else
-                       ret = cgd_ioctl_set(cs, data, l);
-               break;
+                       return EBUSY;
+               return cgd_ioctl_set(cs, data, l);
        case CGDIOCCLR:
-
                if (DK_BUSY(&cs->sc_dksc, pmask))
-                       ret = EBUSY;
-               else
-                       ret = cgd_ioctl_clr(cs, l);
-               break;
-
+                       return EBUSY;
+               return cgd_ioctl_clr(cs, l);
        case DIOCCACHESYNC:
                /*
                 * XXX Do we really need to care about having a writable
@@ -540,15 +531,10 @@ cgdioctl(dev_t dev, u_long cmd, void *da
                /*
                 * We pass this call down to the underlying disk.
                 */
-               ret = VOP_IOCTL(cs->sc_tvn, cmd, data, flag, l->l_cred);
-               break;
-
+               return VOP_IOCTL(cs->sc_tvn, cmd, data, flag, l->l_cred);
        default:
-               ret = dk_ioctl(di, dksc, dev, cmd, data, flag, l);
-               break;
+               return dk_ioctl(di, dksc, dev, cmd, data, flag, l);
        }
-
-       return ret;
 }
 
 static int