Date: Tue, 26 Sep 2023 13:30:04 -0700 From: John Baldwin <jhb@FreeBSD.org> To: Andrew Turner <andrew@fubar.geek.nz> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: f1c5a2e3a625 - main - virtio_random: Pipeline fetching the data Message-ID: <6964df82-7e4a-36a6-bb83-a8985dbeb01d@FreeBSD.org> In-Reply-To: <8B2042EF-728E-4A8A-8878-640B3097191E@fubar.geek.nz> References: <202309051600.385G0Bmk031832@gitrepo.freebsd.org> <8B2042EF-728E-4A8A-8878-640B3097191E@fubar.geek.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
On 9/26/23 7:08 PM, Andrew Turner wrote: > Is there a plan to MFC this to 14? Without it I’m seeing 50% CPU on a dual core arm64 Hetzner VM. > > Andrew I can merge it sure, I've just been AFK most of last week and am still catching up a bit. >> On 5 Sep 2023, at 17:00, John Baldwin <jhb@freebsd.org> wrote: >> >> The branch main has been updated by jhb: >> >> URL: https://cgit.FreeBSD.org/src/commit/?id=f1c5a2e3a625053e2b70d5b1777d849a4d9328f2 >> >> commit f1c5a2e3a625053e2b70d5b1777d849a4d9328f2 >> Author: John-Mark Gurney <jmg@FreeBSD.org> >> AuthorDate: 2023-09-05 15:59:43 +0000 >> Commit: John Baldwin <jhb@FreeBSD.org> >> CommitDate: 2023-09-05 15:59:43 +0000 >> >> virtio_random: Pipeline fetching the data >> >> Queue an initial fetch of data during attach and after every read >> rather than synchronously fetching data and polling for completion. >> >> If data has not been returned from an previous fetch during read, >> just return EAGAIN rather than blocking. >> >> Co-authored-by: John Baldwin <jhb@FreeBSD.org> >> >> Reviewed by: markj >> Differential Revision: https://reviews.freebsd.org/D41656 >> --- >> sys/dev/virtio/random/virtio_random.c | 73 +++++++++++++++++++++-------------- >> 1 file changed, 43 insertions(+), 30 deletions(-) >> >> diff --git a/sys/dev/virtio/random/virtio_random.c b/sys/dev/virtio/random/virtio_random.c >> index c02b5c98cece..d54e2e6b70d4 100644 >> --- a/sys/dev/virtio/random/virtio_random.c >> +++ b/sys/dev/virtio/random/virtio_random.c >> @@ -54,6 +54,8 @@ struct vtrnd_softc { >> struct virtqueue *vtrnd_vq; >> eventhandler_tag eh; >> bool inactive; >> + struct sglist *vtrnd_sg; >> + uint32_t *vtrnd_value; >> }; >> >> static int vtrnd_modevent(module_t, int, void *); >> @@ -67,6 +69,7 @@ static int vtrnd_negotiate_features(struct vtrnd_softc *); >> static int vtrnd_setup_features(struct vtrnd_softc *); >> static int vtrnd_alloc_virtqueue(struct vtrnd_softc *); >> static int vtrnd_harvest(struct vtrnd_softc *, void *, size_t *); >> +static void vtrnd_enqueue(struct vtrnd_softc *sc); >> static unsigned vtrnd_read(void *, unsigned); >> >> #define VTRND_FEATURES 0 >> @@ -138,12 +141,17 @@ static int >> vtrnd_attach(device_t dev) >> { >> struct vtrnd_softc *sc, *exp; >> + size_t len; >> int error; >> >> sc = device_get_softc(dev); >> sc->vtrnd_dev = dev; >> virtio_set_feature_desc(dev, vtrnd_feature_desc); >> >> + len = sizeof(*sc->vtrnd_value) * HARVESTSIZE; >> + sc->vtrnd_value = malloc_aligned(len, len, M_DEVBUF, M_WAITOK); >> + sc->vtrnd_sg = sglist_build(sc->vtrnd_value, len, M_WAITOK); >> + >> error = vtrnd_setup_features(sc); >> if (error) { >> device_printf(dev, "cannot setup features\n"); >> @@ -174,6 +182,8 @@ vtrnd_attach(device_t dev) >> sc->inactive = false; >> random_source_register(&random_vtrnd); >> >> + vtrnd_enqueue(sc); >> + >> fail: >> if (error) >> vtrnd_detach(dev); >> @@ -185,6 +195,7 @@ static int >> vtrnd_detach(device_t dev) >> { >> struct vtrnd_softc *sc; >> + uint32_t rdlen; >> >> sc = device_get_softc(dev); >> KASSERT( >> @@ -197,7 +208,13 @@ vtrnd_detach(device_t dev) >> sc->eh = NULL; >> } >> random_source_deregister(&random_vtrnd); >> + >> + /* clear the queue */ >> + virtqueue_poll(sc->vtrnd_vq, &rdlen); >> + >> atomic_store_explicit(&g_vtrnd_softc, NULL, memory_order_release); >> + sglist_free(sc->vtrnd_sg); >> + zfree(sc->vtrnd_value, M_DEVBUF); >> return (0); >> } >> >> @@ -251,49 +268,45 @@ vtrnd_alloc_virtqueue(struct vtrnd_softc *sc) >> return (virtio_alloc_virtqueues(dev, 0, 1, &vq_info)); >> } >> >> +static void >> +vtrnd_enqueue(struct vtrnd_softc *sc) >> +{ >> + struct virtqueue *vq; >> + int error __diagused; >> + >> + vq = sc->vtrnd_vq; >> + >> + KASSERT(virtqueue_empty(vq), ("%s: non-empty queue", __func__)); >> + >> + error = virtqueue_enqueue(vq, sc, sc->vtrnd_sg, 0, 1); >> + KASSERT(error == 0, ("%s: virtqueue_enqueue returned error: %d", >> + __func__, error)); >> + >> + virtqueue_notify(vq); >> +} >> + >> static int >> vtrnd_harvest(struct vtrnd_softc *sc, void *buf, size_t *sz) >> { >> - struct sglist_seg segs[1]; >> - struct sglist sg; >> struct virtqueue *vq; >> - uint32_t value[HARVESTSIZE] __aligned(sizeof(uint32_t) * HARVESTSIZE); >> + void *cookie; >> uint32_t rdlen; >> - int error; >> - >> - _Static_assert(sizeof(value) < PAGE_SIZE, "sglist assumption"); >> >> if (sc->inactive) >> return (EDEADLK); >> >> - sglist_init(&sg, 1, segs); >> - error = sglist_append(&sg, value, *sz); >> - if (error != 0) >> - panic("%s: sglist_append error=%d", __func__, error); >> - >> vq = sc->vtrnd_vq; >> - KASSERT(virtqueue_empty(vq), ("%s: non-empty queue", __func__)); >> - >> - error = virtqueue_enqueue(vq, buf, &sg, 0, 1); >> - if (error != 0) >> - return (error); >> - >> - /* >> - * Poll for the response, but the command is likely already >> - * done when we return from the notify. >> - */ >> - virtqueue_notify(vq); >> - virtqueue_poll(vq, &rdlen); >> >> - if (rdlen > *sz) >> - panic("%s: random device wrote %zu bytes beyond end of provided" >> - " buffer %p:%zu", __func__, (size_t)rdlen - *sz, >> - (void *)value, *sz); >> - else if (rdlen == 0) >> + cookie = virtqueue_dequeue(vq, &rdlen); >> + if (cookie == NULL) >> return (EAGAIN); >> + KASSERT(cookie == sc, ("%s: cookie mismatch", __func__)); >> + >> *sz = MIN(rdlen, *sz); >> - memcpy(buf, value, *sz); >> - explicit_bzero(value, *sz); >> + memcpy(buf, sc->vtrnd_value, *sz); >> + >> + vtrnd_enqueue(sc); >> + >> return (0); >> } >> >> > -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6964df82-7e4a-36a6-bb83-a8985dbeb01d>