On Thu, Sep 16, 2010 at 03:44:02PM -0400, Loganaden Velvindron wrote:
> Greetings,
>
> From brad, remove superfluous braces.
>
The direction of this diff is OK. The mclgeti() usage seems OK.
What made my alarmbells go off was the removal of xl_rx_resync(). Now that
function in itself is dumb but it made me realize that xl(4) is missing
many if not most bus_dma operations. Especially bus_dmamap_sync() are
missing at various places and xl_rx_resync() was probably a workaround for
it. So I have the feeling that this diff may make xl(4) worse on systems
that require the syncs.
It is best to test drivers not only on i386 or amd64 but also on sparc64
which is a very good system to verify proper bus_dma operation.
> Index: src/sys/dev/ic/xl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/xl.c,v
> retrieving revision 1.96
> diff -u -p -r1.96 xl.c
> --- src/sys/dev/ic/xl.c 7 Sep 2010 16:21:43 -0000 1.96
> +++ src/sys/dev/ic/xl.c 16 Sep 2010 19:36:53 -0000
> @@ -153,7 +153,6 @@ void xl_stats_update(void *);
> int xl_encap(struct xl_softc *, struct xl_chain *,
> struct mbuf * );
> void xl_rxeof(struct xl_softc *);
> -int xl_rx_resync(struct xl_softc *);
> void xl_txeof(struct xl_softc *);
> void xl_txeof_90xB(struct xl_softc *);
> void xl_txeoc(struct xl_softc *);
> @@ -180,6 +179,7 @@ void xl_iff(struct xl_softc *);
> void xl_iff_90x(struct xl_softc *);
> void xl_iff_905b(struct xl_softc *);
> int xl_list_rx_init(struct xl_softc *);
> +void xl_fill_rx_ring(struct xl_softc *);
> int xl_list_tx_init(struct xl_softc *);
> int xl_list_tx_init_90xB(struct xl_softc *);
> void xl_wait(struct xl_softc *);
> @@ -1076,8 +1076,6 @@ xl_list_rx_init(struct xl_softc *sc)
> for (i = 0; i < XL_RX_LIST_CNT; i++) {
> cd->xl_rx_chain[i].xl_ptr =
> (struct xl_list_onefrag *)&ld->xl_rx_list[i];
> - if (xl_newbuf(sc, &cd->xl_rx_chain[i]) == ENOBUFS)
> - return(ENOBUFS);
> if (i == (XL_RX_LIST_CNT - 1))
> n = 0;
> else
> @@ -1088,11 +1086,30 @@ xl_list_rx_init(struct xl_softc *sc)
> ld->xl_rx_list[i].xl_next = htole32(next);
> }
>
> - cd->xl_rx_head = &cd->xl_rx_chain[0];
> -
> + cd->xl_rx_prod = cd->xl_rx_cons = &cd->xl_rx_chain[0];
> + cd->xl_rx_cnt = 0;
> + xl_fill_rx_ring(sc);
> return (0);
> }
>
> +void
> +xl_fill_rx_ring(struct xl_softc *sc)
> +{
> + struct xl_chain_data *cd;
> + struct xl_list_data *ld;
> +
> + cd = &sc->xl_cdata;
> + ld = sc->xl_ldata;
> +
> + while (cd->xl_rx_cnt < XL_RX_LIST_CNT) {
> + if (xl_newbuf(sc, cd->xl_rx_prod) == ENOBUFS)
> + break;
> + cd->xl_rx_prod = cd->xl_rx_prod->xl_next;
> + cd->xl_rx_cnt++;
> + }
> +}
> +
> +
> /*
> * Initialize an RX descriptor and attach an MBUF cluster.
> */
> @@ -1102,15 +1119,10 @@ xl_newbuf(struct xl_softc *sc, struct xl
> struct mbuf *m_new = NULL;
> bus_dmamap_t map;
>
> - MGETHDR(m_new, M_DONTWAIT, MT_DATA);
> - if (m_new == NULL)
> - return (ENOBUFS);
> -
> - MCLGET(m_new, M_DONTWAIT);
> - if (!(m_new->m_flags & M_EXT)) {
> - m_freem(m_new);
> + m_new = MCLGETI(NULL, M_DONTWAIT, &sc->sc_arpcom.ac_if, MCLBYTES);
> +
> + if (!m_new)
> return (ENOBUFS);
> - }
>
> m_new->m_len = m_new->m_pkthdr.len = MCLBYTES;
> if (bus_dmamap_load(sc->sc_dmat, sc->sc_rx_sparemap,
> @@ -1150,32 +1162,6 @@ xl_newbuf(struct xl_softc *sc, struct xl
> return (0);
> }
>
> -int
> -xl_rx_resync(struct xl_softc *sc)
> -{
> - struct xl_chain_onefrag *pos;
> - int i;
> -
> - pos = sc->xl_cdata.xl_rx_head;
> -
> - for (i = 0; i < XL_RX_LIST_CNT; i++) {
> - bus_dmamap_sync(sc->sc_dmat, sc->sc_listmap,
> - ((caddr_t)pos->xl_ptr - sc->sc_listkva),
> - sizeof(struct xl_list),
> - BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
> -
> - if (pos->xl_ptr->xl_status)
> - break;
> - pos = pos->xl_next;
> - }
> -
> - if (i == XL_RX_LIST_CNT)
> - return (0);
> -
> - sc->xl_cdata.xl_rx_head = pos;
> -
> - return (EAGAIN);
> -}
>
> /*
> * A frame has been uploaded: pass the resulting mbuf chain up to
> @@ -1195,12 +1181,14 @@ xl_rxeof(struct xl_softc *sc)
>
> again:
>
> - while ((rxstat = letoh32(sc->xl_cdata.xl_rx_head->xl_ptr->xl_status))
> - != 0) {
> - cur_rx = sc->xl_cdata.xl_rx_head;
> - sc->xl_cdata.xl_rx_head = cur_rx->xl_next;
> + while ((rxstat = letoh32(sc->xl_cdata.xl_rx_cons->xl_ptr->xl_status))
> + != 0 && sc->xl_cdata.xl_rx_cnt > 0) {
> + cur_rx = sc->xl_cdata.xl_rx_cons;
> + m = cur_rx->xl_mbuf;
> + cur_rx->xl_mbuf = NULL;
> + sc->xl_cdata.xl_rx_cons = cur_rx->xl_next;
> + sc->xl_cdata.xl_rx_cnt--;
> total_len = rxstat & XL_RXSTAT_LENMASK;
> -
> bus_dmamap_sync(sc->sc_dmat, sc->sc_listmap,
> ((caddr_t)cur_rx->xl_ptr - sc->sc_listkva),
> sizeof(struct xl_list),
> @@ -1224,6 +1212,7 @@ again:
> if (rxstat & XL_RXSTAT_UP_ERROR) {
> ifp->if_ierrors++;
> cur_rx->xl_ptr->xl_status = htole32(0);
> + m_freem(m);
> continue;
> }
>
> @@ -1237,22 +1226,7 @@ again:
> "packet dropped\n", sc->sc_dev.dv_xname);
> ifp->if_ierrors++;
> cur_rx->xl_ptr->xl_status = htole32(0);
> - continue;
> - }
> -
> - /* No errors; receive the packet. */
> - m = cur_rx->xl_mbuf;
> -
> - /*
> - * Try to conjure up a new mbuf cluster. If that
> - * fails, it means we have an out of memory condition and
> - * should leave the buffer in place and continue. This will
> - * result in a lost packet, but there's little else we
> - * can do in this situation.
> - */
> - if (xl_newbuf(sc, cur_rx) == ENOBUFS) {
> - ifp->if_ierrors++;
> - cur_rx->xl_ptr->xl_status = htole32(0);
> + m_freem(m);
> continue;
> }
>
> @@ -1286,7 +1260,7 @@ again:
>
> ether_input_mbuf(ifp, m);
> }
> -
> + xl_fill_rx_ring(sc);
> /*
> * Handle the 'end of channel' condition. When the upload
> * engine hits the end of the RX ring, it will stall. This
> @@ -1303,13 +1277,11 @@ again:
> CSR_READ_4(sc, XL_UPLIST_STATUS) & XL_PKTSTAT_UP_STALLED) {
> CSR_WRITE_2(sc, XL_COMMAND, XL_CMD_UP_STALL);
> xl_wait(sc);
> - CSR_WRITE_4(sc, XL_UPLIST_PTR,
> - sc->sc_listmap->dm_segs[0].ds_addr +
> - offsetof(struct xl_list_data, xl_rx_list[0]));
> - sc->xl_cdata.xl_rx_head = &sc->xl_cdata.xl_rx_chain[0];
> CSR_WRITE_2(sc, XL_COMMAND, XL_CMD_UP_UNSTALL);
> + xl_fill_rx_ring(sc);
> goto again;
> }
> +
> }
>
> /*
> @@ -1514,16 +1486,9 @@ xl_intr(void *arg)
> if (sc->intr_ack)
> (*sc->intr_ack)(sc);
>
> - if (status & XL_STAT_UP_COMPLETE) {
> - int curpkts;
> -
> - curpkts = ifp->if_ipackets;
> + if (status & XL_STAT_UP_COMPLETE)
> xl_rxeof(sc);
> - if (curpkts == ifp->if_ipackets) {
> - while (xl_rx_resync(sc))
> - xl_rxeof(sc);
> - }
> - }
> +
>
> if (status & XL_STAT_DOWN_COMPLETE) {
> if (sc->xl_type == XL_TYPE_905B)
> @@ -2677,7 +2642,7 @@ xl_attach(struct xl_softc *sc)
> */
> if_attach(ifp);
> ether_ifattach(ifp);
> -
> + m_clsetwms(ifp, MCLBYTES, 2, XL_RX_LIST_CNT - 1);
> sc->sc_sdhook = shutdownhook_establish(xl_shutdown, sc);
> }
>
> Index: src/sys/dev/ic/xlreg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/xlreg.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 xlreg.h
> --- src/sys/dev/ic/xlreg.h 7 Sep 2010 16:21:43 -0000 1.24
> +++ src/sys/dev/ic/xlreg.h 16 Sep 2010 19:36:54 -0000
> @@ -480,7 +480,9 @@ struct xl_chain_data {
> struct xl_chain_onefrag xl_rx_chain[XL_RX_LIST_CNT];
> struct xl_chain xl_tx_chain[XL_TX_LIST_CNT];
>
> - struct xl_chain_onefrag *xl_rx_head;
> + struct xl_chain_onefrag *xl_rx_cons;
> + struct xl_chain_onefrag *xl_rx_prod;
> + int xl_rx_cnt;
>
> /* 3c90x "boomerang" queuing stuff */
> struct xl_chain *xl_tx_head;
>
> //Logan
> C-x-C-c
>
--
:wq Claudio
