Module Name: src Committed By: jdolecek Date: Mon Apr 13 16:29:59 UTC 2020
Modified Files: src/sys/arch/xen/xen: xbd_xenbus.c Log Message: make xbd(4) D_MPSAFE To generate a diff of this commit: cvs rdiff -u -r1.105 -r1.106 src/sys/arch/xen/xen/xbd_xenbus.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/arch/xen/xen/xbd_xenbus.c diff -u src/sys/arch/xen/xen/xbd_xenbus.c:1.105 src/sys/arch/xen/xen/xbd_xenbus.c:1.106 --- src/sys/arch/xen/xen/xbd_xenbus.c:1.105 Sun Apr 12 20:17:36 2020 +++ src/sys/arch/xen/xen/xbd_xenbus.c Mon Apr 13 16:29:59 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: xbd_xenbus.c,v 1.105 2020/04/12 20:17:36 jdolecek Exp $ */ +/* $NetBSD: xbd_xenbus.c,v 1.106 2020/04/13 16:29:59 jdolecek Exp $ */ /* * Copyright (c) 2006 Manuel Bouyer. @@ -50,7 +50,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c,v 1.105 2020/04/12 20:17:36 jdolecek Exp $"); +__KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c,v 1.106 2020/04/13 16:29:59 jdolecek Exp $"); #include "opt_xen.h" @@ -68,6 +68,7 @@ __KERNEL_RCSID(0, "$NetBSD: xbd_xenbus.c #include <sys/systm.h> #include <sys/stat.h> #include <sys/vnode.h> +#include <sys/mutex.h> #include <dev/dkvar.h> @@ -109,7 +110,7 @@ struct xbd_req { } req_rw; struct { int s_error; - volatile int s_done; + int s_done; } req_sync; } u; }; @@ -121,18 +122,20 @@ struct xbd_req { struct xbd_xenbus_softc { struct dk_softc sc_dksc; /* Must be first in this struct */ struct xenbus_device *sc_xbusd; + unsigned int sc_evtchn; struct intrhand *sc_ih; /* Interrupt handler for this instance. */ - - blkif_front_ring_t sc_ring; - - unsigned int sc_evtchn; + kmutex_t sc_lock; + kcondvar_t sc_cache_flush_cv; + kcondvar_t sc_req_cv; + kcondvar_t sc_detach_cv; + kcondvar_t sc_suspend_cv; + blkif_front_ring_t sc_ring; grant_ref_t sc_ring_gntref; struct xbd_req sc_reqs[XBD_RING_SIZE]; SLIST_HEAD(,xbd_req) sc_xbdreq_head; /* list of free requests */ - bool sc_xbdreq_wait; /* special waiting on xbd_req */ int sc_backend_status; /* our status with backend */ #define BLKIF_STATE_DISCONNECTED 0 @@ -202,7 +205,7 @@ const struct bdevsw xbd_bdevsw = { .d_dump = xbddump, .d_psize = xbdsize, .d_discard = nodiscard, - .d_flag = D_DISK + .d_flag = D_DISK | D_MPSAFE }; const struct cdevsw xbd_cdevsw = { @@ -217,7 +220,7 @@ const struct cdevsw xbd_cdevsw = { .d_mmap = nommap, .d_kqfilter = nokqfilter, .d_discard = nodiscard, - .d_flag = D_DISK + .d_flag = D_DISK | D_MPSAFE }; extern struct cfdriver xbd_cd; @@ -268,6 +271,12 @@ xbd_xenbus_attach(device_t parent, devic sc->sc_xbusd = xa->xa_xbusd; sc->sc_xbusd->xbusd_otherend_changed = xbd_backend_changed; + mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_BIO); + cv_init(&sc->sc_cache_flush_cv, "xbdsync"); + cv_init(&sc->sc_req_cv, "xbdreq"); + cv_init(&sc->sc_detach_cv, "xbddetach"); + cv_init(&sc->sc_suspend_cv, "xbdsuspend"); + /* initialize free requests list */ SLIST_INIT(&sc->sc_xbdreq_head); for (i = 0; i < XBD_RING_SIZE; i++) { @@ -313,37 +322,43 @@ static int xbd_xenbus_detach(device_t dev, int flags) { struct xbd_xenbus_softc *sc = device_private(dev); - int bmaj, cmaj, i, mn, rc, s; + int bmaj, cmaj, i, mn, rc; + + DPRINTF(("%s: xbd_detach\n", device_xname(dev))); rc = disk_begindetach(&sc->sc_dksc.sc_dkdev, NULL, dev, flags); if (rc != 0) return rc; - s = splbio(); /* XXXSMP */ - DPRINTF(("%s: xbd_detach\n", device_xname(dev))); + mutex_enter(&sc->sc_lock); 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 && disk_isbusy(&sc->sc_dksc.sc_dkdev)) { - /* XXXSMP */ - tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach", hz/2); + cv_timedwait(&sc->sc_detach_cv, &sc->sc_lock, hz/2); } + mutex_exit(&sc->sc_lock); + /* Trigger state transition with backend */ xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateClosing); + + mutex_enter(&sc->sc_lock); } if ((flags & DETACH_FORCE) == 0) { /* xbd_xenbus_detach already in progress */ - wakeup(xbd_xenbus_detach); /* XXXSMP */ - splx(s); + cv_broadcast(&sc->sc_detach_cv); + mutex_exit(&sc->sc_lock); return EALREADY; } + mutex_exit(&sc->sc_lock); while (xenbus_read_driver_state(sc->sc_xbusd->xbusd_otherend) != XenbusStateClosed) { - /* XXXSMP */ - tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach2", hz/2); + mutex_enter(&sc->sc_lock); + cv_timedwait(&sc->sc_detach_cv, &sc->sc_lock, hz/2); + mutex_exit(&sc->sc_lock); } - splx(s); /* locate the major number */ bmaj = bdevsw_lookup_major(&xbd_bdevsw); @@ -355,6 +370,7 @@ xbd_xenbus_detach(device_t dev, int flag vdevgone(bmaj, mn, mn, VBLK); vdevgone(cmaj, mn, mn, VCHR); } + if (sc->sc_backend_status == BLKIF_STATE_CONNECTED) { /* Delete all of our wedges. */ dkwedge_delall(&sc->sc_dksc.sc_dkdev); @@ -372,10 +388,10 @@ xbd_xenbus_detach(device_t dev, int flag hypervisor_mask_event(sc->sc_evtchn); xen_intr_disestablish(sc->sc_ih); - while (xengnt_status(sc->sc_ring_gntref)) { - /* XXXSMP */ - tsleep(xbd_xenbus_detach, PRIBIO, "xbd_ref", hz/2); - } + mutex_enter(&sc->sc_lock); + while (xengnt_status(sc->sc_ring_gntref)) + cv_timedwait(&sc->sc_detach_cv, &sc->sc_lock, hz/2); + xengnt_revoke_access(sc->sc_ring_gntref); uvm_km_free(kernel_map, (vaddr_t)sc->sc_ring.sring, PAGE_SIZE, UVM_KMF_WIRED); @@ -388,6 +404,8 @@ xbd_xenbus_detach(device_t dev, int flag } } + mutex_destroy(&sc->sc_lock); + evcnt_detach(&sc->sc_cnt_map_unalign); pmf_device_deregister(dev); @@ -398,24 +416,22 @@ xbd_xenbus_detach(device_t dev, int flag static bool xbd_xenbus_suspend(device_t dev, const pmf_qual_t *qual) { - int s; struct xbd_xenbus_softc *sc; sc = device_private(dev); - s = splbio(); /* XXXSMP */ + mutex_enter(&sc->sc_lock); /* wait for requests to complete, then suspend device */ while (sc->sc_backend_status == BLKIF_STATE_CONNECTED && disk_isbusy(&sc->sc_dksc.sc_dkdev)) { - /* XXXSMP */ - tsleep(xbd_xenbus_suspend, PRIBIO, "xbdsuspend", hz/2); + cv_timedwait(&sc->sc_suspend_cv, &sc->sc_lock, hz/2); } hypervisor_mask_event(sc->sc_evtchn); sc->sc_backend_status = BLKIF_STATE_SUSPENDED; xen_intr_disestablish(sc->sc_ih); - splx(s); + mutex_exit(&sc->sc_lock); xenbus_device_suspend(sc->sc_xbusd); aprint_verbose_dev(dev, "removed event channel %d\n", sc->sc_evtchn); @@ -465,8 +481,8 @@ xbd_xenbus_resume(device_t dev, const pm aprint_verbose_dev(dev, "using event channel %d\n", sc->sc_evtchn); - sc->sc_ih = xen_intr_establish_xname(-1, &xen_pic, sc->sc_evtchn, IST_LEVEL, - IPL_BIO, &xbd_handler, sc, false, device_xname(dev)); + sc->sc_ih = xen_intr_establish_xname(-1, &xen_pic, sc->sc_evtchn, + IST_LEVEL, IPL_BIO, &xbd_handler, sc, true, device_xname(dev)); KASSERT(sc->sc_ih != NULL); again: @@ -532,7 +548,6 @@ xbd_backend_changed(void *arg, XenbusSta struct disk_geom *dg; char buf[32]; - int s; DPRINTF(("%s: new backend state %d\n", device_xname(sc->sc_dksc.sc_dev), new_state)); @@ -543,16 +558,15 @@ xbd_backend_changed(void *arg, XenbusSta case XenbusStateInitialised: break; case XenbusStateClosing: - s = splbio(); /* XXXSMP */ + mutex_enter(&sc->sc_lock); 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 && disk_isbusy(&sc->sc_dksc.sc_dkdev)) { - /* XXXSMP */ - tsleep(xbd_xenbus_detach, PRIBIO, "xbddetach", hz/2); + cv_timedwait(&sc->sc_detach_cv, &sc->sc_lock, hz/2); } - splx(s); + mutex_exit(&sc->sc_lock); xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateClosed); break; case XenbusStateConnected: @@ -568,7 +582,6 @@ xbd_backend_changed(void *arg, XenbusSta xbd_connect(sc); sc->sc_shutdown = BLKIF_SHUTDOWN_RUN; - hypervisor_unmask_event(sc->sc_evtchn); sc->sc_xbdsize = sc->sc_sectors * (uint64_t)sc->sc_secsize / DEV_BSIZE; dg = &sc->sc_dksc.sc_dkdev.dk_geom; @@ -586,6 +599,7 @@ xbd_backend_changed(void *arg, XenbusSta disk_attach(&sc->sc_dksc.sc_dkdev); sc->sc_backend_status = BLKIF_STATE_CONNECTED; + hypervisor_unmask_event(sc->sc_evtchn); format_bytes(buf, sizeof(buf), sc->sc_sectors * sc->sc_secsize); aprint_normal_dev(sc->sc_dksc.sc_dev, @@ -680,6 +694,8 @@ xbd_handler(void *arg) if (__predict_false(sc->sc_backend_status != BLKIF_STATE_CONNECTED)) return 0; + + mutex_enter(&sc->sc_lock); again: resp_prod = sc->sc_ring.sring->rsp_prod; xen_rmb(); /* ensure we see replies up to resp_prod */ @@ -691,7 +707,7 @@ again: KASSERT(xbdreq->req_bp == NULL); xbdreq->req_sync.s_error = rep->status; xbdreq->req_sync.s_done = 1; - wakeup(xbdreq); /* XXXSMP */ + cv_broadcast(&sc->sc_cache_flush_cv); /* caller will free the req */ continue; } @@ -742,10 +758,11 @@ again: if (more_to_do) goto again; - if (sc->sc_xbdreq_wait) - wakeup(&sc->sc_xbdreq_wait); /* XXXSMP */ - else - dk_start(&sc->sc_dksc, NULL); + cv_signal(&sc->sc_req_cv); + mutex_exit(&sc->sc_lock); + + dk_start(&sc->sc_dksc, NULL); + return 1; } @@ -856,7 +873,6 @@ xbdioctl(dev_t dev, u_long cmd, void *da device_lookup_private(&xbd_cd, DISKUNIT(dev)); struct dk_softc *dksc; int error; - int s; struct xbd_req *xbdreq; blkif_request_t *req; int notify; @@ -880,47 +896,37 @@ xbdioctl(dev_t dev, u_long cmd, void *da if ((sc->sc_features & BLKIF_FEATURE_CACHE_FLUSH) == 0) return EOPNOTSUPP; - s = splbio(); /* XXXSMP */ - - while (RING_FULL(&sc->sc_ring)) { - sc->sc_xbdreq_wait = 1; - /* XXXSMP */ - tsleep(&sc->sc_xbdreq_wait, PRIBIO, "xbdreq", 0); - } - sc->sc_xbdreq_wait = 0; + mutex_enter(&sc->sc_lock); + while ((xbdreq = SLIST_FIRST(&sc->sc_xbdreq_head)) == NULL) + cv_wait(&sc->sc_req_cv, &sc->sc_lock); + + SLIST_REMOVE_HEAD(&sc->sc_xbdreq_head, req_next); + req = RING_GET_REQUEST(&sc->sc_ring, + sc->sc_ring.req_prod_pvt); + req->id = xbdreq->req_id; + req->operation = BLKIF_OP_FLUSH_DISKCACHE; + req->handle = sc->sc_handle; + xbdreq->req_sync.s_done = 0; + sc->sc_ring.req_prod_pvt++; + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&sc->sc_ring, notify); + if (notify) + hypervisor_notify_via_evtchn(sc->sc_evtchn); + /* request sent, now wait for completion */ + while (xbdreq->req_sync.s_done == 0) + cv_wait(&sc->sc_cache_flush_cv, &sc->sc_lock); + + if (xbdreq->req_sync.s_error == BLKIF_RSP_EOPNOTSUPP) + error = EOPNOTSUPP; + else if (xbdreq->req_sync.s_error == BLKIF_RSP_OKAY) + error = 0; + else + error = EIO; + SLIST_INSERT_HEAD(&sc->sc_xbdreq_head, xbdreq, req_next); + cv_signal(&sc->sc_req_cv); + mutex_exit(&sc->sc_lock); - xbdreq = SLIST_FIRST(&sc->sc_xbdreq_head); - if (__predict_false(xbdreq == NULL)) { - DPRINTF(("xbdioctl: no req\n")); - error = ENOMEM; - } else { - SLIST_REMOVE_HEAD(&sc->sc_xbdreq_head, req_next); - req = RING_GET_REQUEST(&sc->sc_ring, - sc->sc_ring.req_prod_pvt); - req->id = xbdreq->req_id; - req->operation = BLKIF_OP_FLUSH_DISKCACHE; - req->handle = sc->sc_handle; - xbdreq->req_sync.s_done = 0; - sc->sc_ring.req_prod_pvt++; - RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&sc->sc_ring, - notify); - if (notify) - hypervisor_notify_via_evtchn(sc->sc_evtchn); - /* request sent, now wait for completion */ - while (xbdreq->req_sync.s_done == 0) { - /* XXXSMP */ - tsleep(xbdreq, PRIBIO, "xbdsync", 0); - } - if (xbdreq->req_sync.s_error == BLKIF_RSP_EOPNOTSUPP) - error = EOPNOTSUPP; - else if (xbdreq->req_sync.s_error == BLKIF_RSP_OKAY) - error = 0; - else - error = EIO; - SLIST_INSERT_HEAD(&sc->sc_xbdreq_head, xbdreq, - req_next); - } - splx(s); + /* Restart I/O if it was waiting for req */ + dk_start(&sc->sc_dksc, NULL); break; default: @@ -955,12 +961,11 @@ xbd_diskstart(device_t self, struct buf paddr_t ma; int nsects, nbytes, seg; int notify, error = 0; - int s; DPRINTF(("xbd_diskstart(%p): b_bcount = %ld\n", bp, (long)bp->b_bcount)); - s = splbio(); /* XXX SMP */ + mutex_enter(&sc->sc_lock); if (sc->sc_shutdown != BLKIF_SHUTDOWN_RUN) { error = EIO; @@ -982,12 +987,6 @@ xbd_diskstart(device_t self, struct buf goto out; } - if (RING_FULL(&sc->sc_ring) || sc->sc_xbdreq_wait) { - DPRINTF(("xbd_diskstart: ring_full\n")); - error = EAGAIN; - goto out; - } - xbdreq = SLIST_FIRST(&sc->sc_xbdreq_head); if (__predict_false(xbdreq == NULL)) { DPRINTF(("xbd_diskstart: no req\n")); @@ -1048,6 +1047,10 @@ xbd_diskstart(device_t self, struct buf &xbdreq->req_gntref[seg]))) { printf("%s: %s: xengnt_grant_access failed", device_xname(sc->sc_dksc.sc_dev), __func__); + bus_dmamap_unload(sc->sc_xbusd->xbusd_dmat, + xbdreq->req_dmamap); + SLIST_INSERT_HEAD(&sc->sc_xbdreq_head, xbdreq, + req_next); error = EFAULT; goto out; } @@ -1062,20 +1065,18 @@ xbd_diskstart(device_t self, struct buf hypervisor_notify_via_evtchn(sc->sc_evtchn); out: - splx(s); /* XXXSMP */ + mutex_exit(&sc->sc_lock); return error; } static int xbd_map_align(struct xbd_req *req) { - int s = splvm(); /* XXXSMP - bogus? */ int rc; rc = uvm_km_kmem_alloc(kmem_va_arena, req->req_bp->b_bcount, (VM_NOSLEEP | VM_INSTANTFIT), (vmem_addr_t *)&req->req_data); - splx(s); if (__predict_false(rc != 0)) return ENOMEM; if ((req->req_bp->b_flags & B_READ) == 0) @@ -1087,11 +1088,8 @@ xbd_map_align(struct xbd_req *req) static void xbd_unmap_align(struct xbd_req *req) { - int s; if (req->req_bp->b_flags & B_READ) memcpy(req->req_bp->b_data, req->req_data, req->req_bp->b_bcount); - s = splvm(); /* XXXSMP - bogus? */ uvm_km_kmem_free(kmem_va_arena, (vaddr_t)req->req_data, req->req_bp->b_bcount); - splx(s); }