Date: Fri, 12 Oct 2007 03:07:39 GMT From: Kip Macy <kmacy@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 127403 for review Message-ID: <200710120307.l9C37dOv056981@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=127403 Change 127403 by kmacy@kmacy_home:ethng on 2007/10/12 03:06:45 fix handling of case where packet header mbuf contained zero bytes of data - don't modify initial mbuf chain while iterating through it - when collapsing mbufs do it from the array and not the chain - when freeing mbufs do it from the chain and not the array skip special handling of mbuf chain with only buffer but multiple mbufs probably not common enough to special case and the way it was being handled could leak memory Affected files ... .. //depot/projects/ethng/src/sys/dev/cxgb/sys/uipc_mvec.c#15 edit Differences ... ==== //depot/projects/ethng/src/sys/dev/cxgb/sys/uipc_mvec.c#15 (text+ko) ==== @@ -125,7 +125,6 @@ mi->mi_flags = m->m_flags; mi->mi_len = m->m_len; - if (m->m_flags & M_PKTHDR) { mi->mi_ether_vtag = m->m_pkthdr.ether_vtag; @@ -141,6 +140,7 @@ mi->mi_size = (m->m_type == EXT_CLIOVEC) ? MCLBYTES : MIOVBYTES; mi->mi_type = m->m_type; mi->mi_len = m->m_pkthdr.len; + KASSERT(mi->mi_len, ("empty packet")); mi->mi_refcnt = NULL; } else if (m->m_flags & M_EXT) { memcpy(&mi->mi_ext, &m->m_ext, sizeof(struct m_ext_)); @@ -171,11 +171,12 @@ int busdma_map_sg_collapse(struct mbuf **m, bus_dma_segment_t *segs, int *nsegs) { - struct mbuf *m0, *n = *m; + struct mbuf *m0, *n = *m, *mhead; struct mbuf_iovec *mi; struct mbuf *marray[TX_MAX_SEGS]; int i, type, seg_count, defragged = 0, err = 0; struct mbuf_vec *mv; + uma_zone_t zone; if (n->m_flags & M_PKTHDR && !SLIST_EMPTY(&n->m_pkthdr.tags)) m_tag_delete_chain(n, NULL); @@ -221,20 +222,23 @@ /* * firmware doesn't like empty segments */ - if (__predict_false(n->m_len == 0)) { - n = m_free(n); - if (seg_count) - marray[seg_count - 1]->m_next = n; - continue; - } - seg_count++; + if (__predict_true(n->m_len != 0)) + seg_count++; + n = n->m_next; } +#if 0 + /* + * XXX needs more careful consideration + */ if (__predict_false(seg_count == 1)) { - n = *m; + n = marray[0]; + if (n != *m) + /* XXX */ goto retry; } +#endif if (seg_count == 0) { if (cxgb_debug) printf("empty segment chain\n"); @@ -257,43 +261,41 @@ goto err_out; } if (seg_count > MAX_CL_IOV) { - if ((m0 = uma_zalloc_arg(zone_jumbop, NULL, M_NOWAIT)) == NULL) { - err = ENOMEM; - goto err_out; - } + zone = zone_jumbop; type = EXT_JMPIOVEC; } else if (seg_count > MAX_MIOVEC_IOV) { - DPRINTF("seg count=%d ", seg_count); - if ((m0 = uma_zalloc_arg(zone_clust, NULL, M_NOWAIT)) == NULL) { - err = ENOMEM; - goto err_out; - } + zone = zone_clust; type = EXT_CLIOVEC; } else { - if ((m0 = uma_zalloc_arg(zone_miovec, NULL, M_NOWAIT)) == NULL) { - err = ENOMEM; - goto err_out; - } type = EXT_IOVEC; + zone = zone_miovec; } + if ((m0 = uma_zalloc_arg(zone, NULL, M_NOWAIT)) == NULL) { + err = ENOMEM; + goto err_out; + } - n = marray[0]; - memcpy(m0, n, sizeof(struct m_hdr) + sizeof(struct pkthdr)); + memcpy(m0, *m, sizeof(struct m_hdr) + sizeof(struct pkthdr)); m0->m_type = type; mv = mtomv(m0); mv->mv_count = seg_count; mv->mv_first = 0; - - for (mi = mv->mv_vec; n; mi++, segs++) { + for (i = 0, mi = mv->mv_vec; i < seg_count; mi++, segs++, i++) { + n = marray[i]; busdma_map_mbuf_fast(n, segs); - n = _mcl_collapse_mbuf(mi, n); + _mcl_collapse_mbuf(mi, n); } - for (i = 0; i < seg_count; i++) { - marray[i]->m_next = marray[i]->m_nextpkt = NULL; - if ((marray[i]->m_flags & (M_EXT|M_NOFREE)) == M_EXT) { - marray[i]->m_flags &= ~M_EXT; - m_free(marray[i]); + n = *m; + while (n) { + if (((n->m_flags & (M_EXT|M_NOFREE)) == M_EXT) && (n->m_len > 0)) + n->m_flags &= ~M_EXT; + else if (n->m_len > 0) { + n = n->m_next; + continue; } + mhead = n->m_next; + m_free(n); + n = mhead; } *nsegs = seg_count; *m = m0;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200710120307.l9C37dOv056981>