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

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

The following reply was made to PR kern/52211; it has been noted by GNATS.

From: Masanobu SAITOH <msaitoh%execsw.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, kern-bug-people%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, hannken%eis.cs.tu-bs.de@localhost
Cc: msaitoh%execsw.org@localhost, Robert Elz <kre%munnari.OZ.AU@localhost>
Subject: Re: kern/52211: vioif stops on dmamap load error
Date: Fri, 12 May 2017 18:44:12 +0900

 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)