Hi,
I have committed the vio diff with some tweaks to remove some more now
unused code. But I think this diff is actually incorrect:
On Tue, 31 Mar 2015, Kimberley Manning wrote:
> Index: if_pcn.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_pcn.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 if_pcn.c
> --- if_pcn.c 14 Mar 2015 03:38:48 -0000 1.36
> +++ if_pcn.c 27 Mar 2015 12:17:24 -0000
> @@ -851,25 +851,23 @@ pcn_start(struct ifnet *ifp)
> * were short on resources. In this case, we'll copy
> * and try again.
> */
> - if (bus_dmamap_load_mbuf(sc->sc_dmat, dmamap, m0,
> - BUS_DMA_WRITE|BUS_DMA_NOWAIT) != 0) {
> - MGETHDR(m, M_DONTWAIT, MT_DATA);
> - if (m == NULL)
> - break;
> - if (m0->m_pkthdr.len > MHLEN) {
> - MCLGET(m, M_DONTWAIT);
> - if ((m->m_flags & M_EXT) == 0) {
> - m_freem(m);
> - break;
> - }
> - }
> - m_copydata(m0, 0, m0->m_pkthdr.len, mtod(m, caddr_t));
> - m->m_pkthdr.len = m->m_len = m0->m_pkthdr.len;
> - error = bus_dmamap_load_mbuf(sc->sc_dmat, dmamap,
> - m, BUS_DMA_WRITE|BUS_DMA_NOWAIT);
> - if (error)
> - break;
> - }
> + error = bus_dmamap_load_mbuf(sc->sc_dmat, dmamap, m0,
> + BUS_DMA_WRITE|BUS_DMA_NOWAIT);
> + switch (error) {
> + case 0:
> + break;
> + case EFBIG:
> + if ((error = m_defrag(m0, M_DONTWAIT)) == 0 &&
> + (error = bus_dmamap_load_mbuf(sc->sc_dmat,
> dmamap,
> + m0, BUS_DMA_WRITE|BUS_DMA_NOWAIT)) == 0)
> + break;
> +
> + /* FALLTHROUGH */
> + default:
> + IFQ_DEQUEUE(&ifp->if_snd, m0);
> + m_freem(m);
m is always NULL. Therefore you leak m0 in the error path. You should
remove the m variable which is no longer necessary, and possibly rename m0
to m. But the rename should probably be done in a second diff for easier
review.
> + continue;
> + }
Other drivers don't dequeue mbufs in out of mem situations. I think you
should just set IFF_OACTIVE and break out of the for loop (not shown in
the diff). The old code just does the break, but I think setting
IFF_OACTIVE would be correct here.
>
> /*
> * Ensure we have enough descriptors free to describe
>
>
