Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/pci refactor error handling in vioscsi_scsipi_reques...



details:   https://anonhg.NetBSD.org/src/rev/f4c913727ed8
branches:  trunk
changeset: 823890:f4c913727ed8
user:      jdolecek <jdolecek%NetBSD.org@localhost>
date:      Sat May 13 20:35:20 2017 +0000

description:
refactor error handling in vioscsi_scsipi_request() to avoid the goto maze,
and add missing virtio_enqueue_abort() call when bus_dmamap_load() fails;
abort is only done implicitely when virtio_enqueue_reserve() fails,
must do it explicitely if there is other reason

also now always print error when bus_dmamap_load() or virtio_enqueue_reserve()
fail - the former shouldn't fail due to BUS_DMA_ALLOCNOW, and the latter
shouldn't ever fail now after the maxnsegs fix for virtio_alloc_vq(), so
error there means driver bug

diffstat:

 sys/dev/pci/vioscsi.c |  40 +++++++++++++++++++++++-----------------
 1 files changed, 23 insertions(+), 17 deletions(-)

diffs (71 lines):

diff -r 39d37a097e1a -r f4c913727ed8 sys/dev/pci/vioscsi.c
--- a/sys/dev/pci/vioscsi.c     Sat May 13 20:17:42 2017 +0000
+++ b/sys/dev/pci/vioscsi.c     Sat May 13 20:35:20 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vioscsi.c,v 1.17 2017/05/13 20:17:42 jdolecek Exp $    */
+/*     $NetBSD: vioscsi.c,v 1.18 2017/05/13 20:35:20 jdolecek Exp $    */
 /*     $OpenBSD: vioscsi.c,v 1.3 2015/03/14 03:38:49 jsg Exp $ */
 
 /*
@@ -18,7 +18,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vioscsi.c,v 1.17 2017/05/13 20:17:42 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vioscsi.c,v 1.18 2017/05/13 20:35:20 jdolecek Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -362,20 +362,23 @@
 
        error = bus_dmamap_load(virtio_dmat(vsc), vr->vr_data,
            xs->data, xs->datalen, NULL, XS2DMA(xs));
-       switch (error) {
-       case 0:
-               break;
-       case ENOMEM:
-       case EAGAIN:
-               xs->error = XS_RESOURCE_SHORTAGE;
-               goto nomore;
-       default:
-               aprint_error_dev(sc->sc_dev, "error %d loading DMA map\n",
-                   error);
+       if (error) {
+               aprint_error_dev(sc->sc_dev, "%s: error %d loading DMA map\n",
+                   __func__, error);
+
+               if (error == ENOMEM || error == EAGAIN) {
+                       /*
+                        * Map is allocated with ALLOCNOW, so this should
+                        * actually never ever happen.
+                        */
+                       xs->error = XS_RESOURCE_SHORTAGE;
+               } else {
 stuffup:
-               xs->error = XS_DRIVER_STUFFUP;
-nomore:
-               /* nothing else to free */
+                       /* not a temporary condition */
+                       xs->error = XS_DRIVER_STUFFUP;
+               }
+
+               virtio_enqueue_abort(vsc, vq, slot);
                scsipi_done(xs);
                return;
        }
@@ -386,10 +389,13 @@
 
        error = virtio_enqueue_reserve(vsc, vq, slot, nsegs);
        if (error) {
-               DPRINTF(("%s: error reserving %d\n", __func__, error));
+               aprint_error_dev(sc->sc_dev, "error reserving %d (nsegs %d)\n",
+                   error, nsegs);
                bus_dmamap_unload(virtio_dmat(vsc), vr->vr_data);
+               /* slot already freed by virtio_enqueue_reserve() */
                xs->error = XS_RESOURCE_SHORTAGE;
-               goto nomore;
+               scsipi_done(xs);
+               return;
        }
 
        vr->vr_xs = xs;



Home | Main Index | Thread Index | Old Index