status of AVILA and CAMBRIA code

[Ian Lepore]

On Mon, 2014-03-03 at 06:06 +0200, John Hay wrote:
> On Sun, Mar 02, 2014 at 08:52:37PM -0700, Ian Lepore wrote:
> > On Sat, 2014-03-01 at 12:44 +0200, John Hay wrote:
> > > On Mon, Feb 24, 2014 at 03:26:14PM +0200, John Hay wrote:
> > > > Hi Ian,
> > > > 
> > > > On Fri, Feb 21, 2014 at 09:47:29AM -0700, Ian Lepore wrote:
> > > > > On Fri, 2014-02-21 at 15:05 +0200, John Hay wrote:
> > > > > > On Wed, Feb 19, 2014 at 09:29:38AM -0800, John-Mark Gurney wrote:
> > > > > > > John Hay wrote this message on Wed, Feb 19, 2014 at 12:59 +0200:
> > > > > > > > What is the status of AVILA and CAMBRIA builds in our tree? Have anybody
> > > > > > > > had success recently? From the lists I can see that other people have
> > > > > > > > also asked in September 2013, but I cannot figure out if there was any
> > > > > > > > successes at that stage. :-)
> > > > > > ...
> > > > > > > > 
> > > > > > > > Somewhere along the line the ethernet npe0 device also broke, writing
> > > > > > > > "npe0: npestart_locked: too many fragments 0". But I did not test the
> > > > > > > > npe0 device with every build and only realised it this morning, so I
> > > > > > > > do not know where it broke. :-)
> > > 
> > > It looks like adding a few bus_dmamap_unload()s in if_npe.c fixed it. Here is
> > > my patch that seems to fix it. Does it look ok? Can I commit it?
> > > 
> > > Index: xscale/ixp425/if_npe.c
> > > ===================================================================
> > > --- xscale/ixp425/if_npe.c	(revision 246713)
> > > +++ xscale/ixp425/if_npe.c	(working copy)
> > > @@ -1037,6 +1037,8 @@
> > >  
> > >  		sc = npes[NPE_QM_Q_NPE(entry)];
> > >  		npe = P2V(NPE_QM_Q_ADDR(entry), &sc->txdma);
> > > +		bus_dmamap_sync(sc->txdma.mtag, npe->ix_map, BUS_DMASYNC_POSTWRITE);
> > > +		bus_dmamap_unload(sc->txdma.mtag, npe->ix_map);
> > >  		m_freem(npe->ix_m);
> > >  		npe->ix_m = NULL;
> > >  
> > > @@ -1112,6 +1114,12 @@
> > >  
> > >  		DPRINTF(sc, "%s: entry 0x%x neaddr 0x%x ne_len 0x%x\n",
> > >  		    __func__, entry, npe->ix_neaddr, npe->ix_hw->ix_ne[0].len);
> > > +
> > > +		/* Flush mbuf memory for rx'd data */
> > > +		bus_dmamap_sync(dma->mtag, npe->ix_map,
> > > +		    BUS_DMASYNC_POSTREAD);
> > > +		bus_dmamap_unload(dma->mtag, npe->ix_map);
> > > +
> > >  		/*
> > >  		 * Allocate a new mbuf to replenish the rx buffer.
> > >  		 * If doing so fails we drop the rx'd frame so we
> > > @@ -1126,10 +1134,6 @@
> > >  			struct npehwbuf *hw = npe->ix_hw;
> > >  			struct ifnet *ifp = sc->sc_ifp;
> > >  
> > > -			/* Flush mbuf memory for rx'd data */
> > > -			bus_dmamap_sync(dma->mtag, npe->ix_map,
> > > -			    BUS_DMASYNC_POSTREAD);
> > > -
> > >  			/* XXX flush hw buffer; works now 'cuz coherent */
> > >  			/* set m_len etc. per rx frame size */
> > >  			mrx->m_len = be32toh(hw->ix_ne[0].len) & 0xffff;
> > > 
> > > John
> > 
> > I think this is more correct, but one more thing is needed... it looks
> > like design here was to load a map then not unload it until the next
> > time you're about to load it.  I think that's a bad idea and your
> > changes are the better way, but there are still a few unload calls
> > scattered around that now need to be deleted because they would be
> > unloading a map that was already unloaded when the IO completed.
> 
> Thanks, I'll look into the other unloads. I get the feeling that the
> design was actually to only unload if the interface is stopped which
> worked fine until map->sync_count was introduced in arm/busdma_machdep.c?
> 
> Regards
> 
> John

In both the places where bus_dmamap_load_mbuf_sg() is called, an unload
is done right before the load.  It's almost like an attempt to hoard
resources lest something else grab them while the driver is idle. :)

-- Ian