Port-xen archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: xbd detachment
On Tue, Jul 28, 2009 at 06:31:31PM +0200, Manuel Bouyer wrote:
> On Sat, Jul 25, 2009 at 03:50:39PM -0500, David Young wrote:
> > > > Should this routine follow some other protocol in order to close down
> > > > and revoke the grant_ref_t?
> > >
> > > Hardly, revocation will end in panic() (you cannot free a grant table
> > > entry while there is a read/write lock acquired by the other end on the
> > > referenced page).
> >
> > What I mean is that xbd_xenbus_detach() may not use the correct
> > protocol, and that is why the transfer/write/read status does not
> > clear. For example, what if xbd(4) indicates to the dom0 that it is
> > finished with the ring, and then queues a transfer on the ring due to a
> > programming mistake?
> >
> > It seems to me that xbd_xenbus_detach() may not be used very much, if at
> > all. Moreover, if it has ever been used before, then I think that the
> > Dom0, not the DomU, had initiated the device's detachment. That may, in
> > fact, make a difference, even if it should not.
>
> Actually it does. xbd_xenbus_detach() is called by the xenbus handler
> though config_detach(); at this point the backed has switched to
> XenbusStateClosing then XenbusStateClosed.
> For a locally-initiated detach, the frontend need to switch to
> XenbusStateClosing, then wait for the backend to switch to XenbusStateClosing
> then XenbusStateClosed. This will cause config_detach() to be called
> a second time.
>
> Would the (untested) attached patch help ?
Thanks, that patch was enormously helpful.
I got it to mostly work with one minor change, except that
config_detach(,DETACH_FORCE) panicked: config_detach() really does not
like for a driver's detachment routine to return != 0 if the detachment
was forced. So I made it so xbd_xenbus_detach(,DETACH_FORCE) waits for
the detach to complete, while xbd_xenbus_detach(,!DETACH_FORCE) exits
with EALREADY. That seems to work fine.
See the attached patch. Ok to commit?
Included in my patch are some changes to the other xbd driver, whose
sources are in xen/xbd.c. Is that driver only applicable to Xen 2? I
still need to test that patch....
Dave
--
David Young OJC Technologies
dyoung%ojctech.com@localhost Urbana, IL * (217) 278-3933
Index: xen/xbd.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xbd.c,v
retrieving revision 1.52
diff -p -u -u -p -r1.52 xbd.c
--- xen/xbd.c 21 Jan 2009 20:40:40 -0000 1.52
+++ xen/xbd.c 28 Jul 2009 23:15:50 -0000
@@ -82,32 +82,32 @@ static int xbd_detach(device_t, int);
#if NXBD_HYPERVISOR > 0
int xbd_match(device_t, cfdata_t, void *);
-CFATTACH_DECL_NEW(xbd_hypervisor, sizeof(struct xbd_softc),
- xbd_match, xbd_attach, xbd_detach, NULL);
+CFATTACH_DECL3_NEW(xbd_hypervisor, sizeof(struct xbd_softc),
+ xbd_match, xbd_attach, xbd_detach, NULL, NULL, NULL, DVF_DETACH_SHUTDOWN);
extern struct cfdriver xbd_cd;
#endif
#if NWD > 0
int xbd_wd_match(device_t, cfdata_t, void *);
-CFATTACH_DECL_NEW(wd_xen, sizeof(struct xbd_softc),
- xbd_wd_match, xbd_attach, xbd_detach, NULL);
+CFATTACH_DECL3_NEW(wd_xen, sizeof(struct xbd_softc),
+ xbd_wd_match, xbd_attach, xbd_detach, NULL, NULL, NULL,
DVF_DETACH_SHUTDOWN);
extern struct cfdriver wd_cd;
#endif
#if NSD > 0
int xbd_sd_match(device_t, cfdata_t, void *);
-CFATTACH_DECL_NEW(sd_xen, sizeof(struct xbd_softc),
- xbd_sd_match, xbd_attach, xbd_detach, NULL);
+CFATTACH_DECL3_NEW(sd_xen, sizeof(struct xbd_softc),
+ xbd_sd_match, xbd_attach, xbd_detach, NULL, NULL, NULL,
DVF_DETACH_SHUTDOWN);
extern struct cfdriver sd_cd;
#endif
#if NCD > 0
int xbd_cd_match(device_t, cfdata_t, void *);
-CFATTACH_DECL_NEW(cd_xen, sizeof(struct xbd_softc),
- xbd_cd_match, xbd_attach, xbd_detach, NULL);
+CFATTACH_DECL3_NEW(cd_xen, sizeof(struct xbd_softc),
+ xbd_cd_match, xbd_attach, xbd_detach, NULL, NULL, NULL,
DVF_DETACH_SHUTDOWN);
extern struct cfdriver cd_cd;
#endif
@@ -1163,7 +1163,10 @@ static int
xbd_detach(device_t dv, int flags)
{
struct xbd_softc *xs = device_private(dv);
- int bmaj, cmaj, mn, i;
+ int bmaj, cmaj, i, mn, rc;
+
+ if ((rc = disk_begindetach(&xs->sc_dksc, NULL, dv, flags)) != 0)
+ return rc;
/*
* Mark disk about to be removed (between now and when the xs
Index: xen/xbd_xenbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xbd_xenbus.c,v
retrieving revision 1.40
diff -p -u -u -p -r1.40 xbd_xenbus.c
--- xen/xbd_xenbus.c 15 Jun 2009 21:11:24 -0000 1.40
+++ xen/xbd_xenbus.c 28 Jul 2009 23:15:50 -0000
@@ -110,6 +110,9 @@ struct xbd_xenbus_softc {
#define BLKIF_STATE_CONNECTED 1
#define BLKIF_STATE_SUSPENDED 2
int sc_shutdown;
+#define BLKIF_SHUTDOWN_RUN 0 /* no shutdown */
+#define BLKIF_SHUTDOWN_REMOTE 1 /* backend-initiated shutdown in progress */
+#define BLKIF_SHUTDOWN_LOCAL 2 /* locally-initiated shutdown in progress */
uint64_t sc_sectors; /* number of sectors for this device */
u_long sc_secsize; /* sector size */
@@ -140,8 +143,9 @@ static void xbd_connect(struct xbd_xenbu
static int xbd_map_align(struct xbd_req *);
static void xbd_unmap_align(struct xbd_req *);
-CFATTACH_DECL_NEW(xbd_xenbus, sizeof(struct xbd_xenbus_softc),
- xbd_xenbus_match, xbd_xenbus_attach, xbd_xenbus_detach, NULL);
+CFATTACH_DECL3_NEW(xbd_xenbus, sizeof(struct xbd_xenbus_softc),
+ xbd_xenbus_match, xbd_xenbus_attach, xbd_xenbus_detach, NULL, NULL, NULL,
+ DVF_DETACH_SHUTDOWN);
dev_type_open(xbdopen);
dev_type_close(xbdclose);
@@ -249,7 +253,7 @@ xbd_xenbus_attach(device_t parent, devic
}
sc->sc_backend_status = BLKIF_STATE_DISCONNECTED;
- sc->sc_shutdown = 1;
+ sc->sc_shutdown = BLKIF_SHUTDOWN_REMOTE;
/* initialise shared structures and tell backend that we are ready */
xbd_xenbus_resume(sc);
@@ -263,16 +267,31 @@ static int
xbd_xenbus_detach(device_t dev, int flags)
{
struct xbd_xenbus_softc *sc = device_private(dev);
- int s, bmaj, cmaj, i, mn;
+ int bmaj, cmaj, i, mn, rc, s;
+
+ rc = disk_begindetach(&sc->sc_dksc.sc_dkdev, NULL, dev, flags);
+ if (rc != 0)
+ return rc;
+
s = splbio();
DPRINTF(("%s: xbd_detach\n", device_xname(dev)));
- if (sc->sc_shutdown == 0) {
- sc->sc_shutdown = 1;
+ if (sc->sc_shutdown == BLKIF_SHUTDOWN_RUN) {
+ sc->sc_shutdown = BLKIF_SHUTDOWN_LOCAL;
/* wait for requests to complete */
while (sc->sc_backend_status == BLKIF_STATE_CONNECTED &&
sc->sc_dksc.sc_dkdev.dk_stats->io_busy > 0)
tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach", hz/2);
+ xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateClosing);
}
+ if ((flags & DETACH_FORCE) == 0) {
+ /* xbd_xenbus_detach already in progress */
+ wakeup(xbd_xenbus_detach);
+ splx(s);
+ return EALREADY;
+ }
+ while (xenbus_read_driver_state(sc->sc_xbusd->xbusd_otherend)
+ != XenbusStateClosed)
+ tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach2", hz/2);
splx(s);
/* locate the major number */
@@ -414,12 +433,12 @@ static void xbd_backend_changed(void *ar
break;
case XenbusStateClosing:
s = splbio();
- sc->sc_shutdown = 1;
+ if (sc->sc_shutdown == BLKIF_SHUTDOWN_RUN)
+ sc->sc_shutdown = BLKIF_SHUTDOWN_REMOTE;
/* wait for requests to complete */
while (sc->sc_backend_status == BLKIF_STATE_CONNECTED &&
sc->sc_dksc.sc_dkdev.dk_stats->io_busy > 0)
- tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach",
- hz/2);
+ tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach", hz/2);
splx(s);
xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateClosed);
break;
@@ -434,7 +453,7 @@ static void xbd_backend_changed(void *ar
return;
xbd_connect(sc);
- sc->sc_shutdown = 0;
+ sc->sc_shutdown = BLKIF_SHUTDOWN_RUN;
hypervisor_enable_event(sc->sc_evtchn);
sc->sc_xbdsize =
@@ -635,7 +654,7 @@ xbdstrategy(struct buf *bp)
DPRINTF(("xbdstrategy(%p): b_bcount = %ld\n", bp,
(long)bp->b_bcount));
- if (sc == NULL || sc->sc_shutdown) {
+ if (sc == NULL || sc->sc_shutdown != BLKIF_SHUTDOWN_RUN) {
bp->b_error = EIO;
biodone(bp);
return;
@@ -659,7 +678,7 @@ xbdsize(dev_t dev)
DPRINTF(("xbdsize(%d)\n", dev));
sc = device_lookup_private(&xbd_cd, DISKUNIT(dev));
- if (sc == NULL || sc->sc_shutdown)
+ if (sc == NULL || sc->sc_shutdown != BLKIF_SHUTDOWN_RUN)
return -1;
return dk_size(sc->sc_di, &sc->sc_dksc, dev);
}
@@ -750,7 +769,7 @@ xbdstart(struct dk_softc *dksc, struct b
DPRINTF(("xbdstart(%p): b_bcount = %ld\n", bp, (long)bp->b_bcount));
sc = device_lookup_private(&xbd_cd, DISKUNIT(bp->b_dev));
- if (sc == NULL || sc->sc_shutdown) {
+ if (sc == NULL || sc->sc_shutdown != BLKIF_SHUTDOWN_RUN) {
bp->b_error = EIO;
goto err;
}
Home |
Main Index |
Thread Index |
Old Index