Re: kern/52211: vioif stops on dmamap load error

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

Hi.

On 2017/05/11 19:05, J. Hannken-Illjes wrote:
> The following reply was made to PR kern/52211; it has been noted by GNATS.
>
> From: "J. Hannken-Illjes" <hannken%eis.cs.tu-bs.de@localhost>
> To: gnats-bugs%NetBSD.org@localhost
> Cc:
> Subject: Re: kern/52211: vioif stops on dmamap load error
> Date: Thu, 11 May 2017 12:02:15 +0200
>
>   > On 11. May 2017, at 11:26, Masanobu SAITOH <msaitoh%execsw.org@localhost> wrote:
>   >
>   > New one:
>   >
>   > Index: if_vioif.c
>   > ===================================================================
>   > RCS file: /cvsroot/src/sys/dev/pci/if_vioif.c,v
>   > retrieving revision 1.34
>   > diff -u -p -r1.34 if_vioif.c
>   > --- if_vioif.c	28 Mar 2017 04:10:33 -0000	1.34
>   > +++ if_vioif.c	11 May 2017 09:20:00 -0000
>   > @@ -812,6 +812,7 @@ vioif_start(struct ifnet *ifp)
>   >  	for (;;) {
>   > 		int slot, r;
>   > +		struct mbuf *newm;
>   >  		IFQ_DEQUEUE(&ifp->if_snd, m);
>   > @@ -833,7 +834,23 @@ retry:
>   > 		r = bus_dmamap_load_mbuf(virtio_dmat(vsc),
>   > 					 sc->sc_tx_dmamaps[slot],
>   > 					 m, BUS_DMA_WRITE|BUS_DMA_NOWAIT);
>   > -		if (r != 0) {
>   > +		switch (r) {
>   > +		case 0:
>   > +			break;
>   > +		case EFBIG:
>   > +			printf("%s: loadup_mbuf() returned EFBIG (%d segs)\n",
>   > +			    device_xname(sc->sc_dev),
>   > +			    sc->sc_tx_dmamaps[slot]->dm_nsegs);
>   > +			newm = m_defrag(m, M_NOWAIT);
>   > +			if ((newm != NULL) &&
>   > +			    (bus_dmamap_load_mbuf(virtio_dmat(vsc),
>   > +				sc->sc_tx_dmamaps[slot],
>   > +				newm, BUS_DMA_WRITE|BUS_DMA_NOWAIT) == 0)) {
>   > +				m = newm;
>   > +				break;
>   > +			}
>   > +			/* FALLTHROUGH */
>   > +		default:
>   > 			virtio_enqueue_abort(vsc, vq, slot);
>   > 			aprint_error_dev(sc->sc_dev,
>   > 			    "tx dmamap load failed, error code %d\n", r);
>   
>   The machine is running this patch now -- it may take weeks to trigger.
>   
>   We don't break the outer loop on error as the "break" after default
>   now breaks the case and not the for loop but I hope it doesn't harm.

 Oops.

 Please use the following new diff:

Index: if_vioif.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_vioif.c,v
retrieving revision 1.34
diff -u -p -r1.34 if_vioif.c
--- if_vioif.c	28 Mar 2017 04:10:33 -0000	1.34
+++ if_vioif.c	12 May 2017 09:29:19 -0000
@@ -812,6 +812,7 @@ vioif_start(struct ifnet *ifp)
 
 	for (;;) {
 		int slot, r;
+		bool remap = true;
 
 		IFQ_DEQUEUE(&ifp->if_snd, m);
 
@@ -830,12 +831,30 @@ retry:
 		}
 		if (r != 0)
 			panic("enqueue_prep for a tx buffer");
+retry_load:
 		r = bus_dmamap_load_mbuf(virtio_dmat(vsc),
 					 sc->sc_tx_dmamaps[slot],
 					 m, BUS_DMA_WRITE|BUS_DMA_NOWAIT);
-		if (r != 0) {
+		if ((r == EFBIG) && remap) {
+			struct mbuf *newm;
+
+			device_printf(sc->sc_dev,
+			    "loadup_mbuf() returned EFBIG (%d segs)\n",
+			    sc->sc_tx_dmamaps[slot]->dm_nsegs);
+			remap = false;
+			newm = m_defrag(m, M_NOWAIT);
+			if (newm == NULL) {
+				virtio_enqueue_abort(vsc, vq, slot);
+				device_printf(sc->sc_dev,
+				    "m_defrag() failed\n");
+				break;
+			} else {
+				m = newm;
+				goto retry_load;
+			}
+		} else {
 			virtio_enqueue_abort(vsc, vq, slot);
-			aprint_error_dev(sc->sc_dev,
+			device_printf(sc->sc_dev,
 			    "tx dmamap load failed, error code %d\n", r);
 			break;
 		}




>   > BTW, what hypervisor are you using?
>   
>   KVM from CentOS 6.8.
>   
>   --
>   J. Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig (Germany)
>   


--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)