Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 Jan 2014 22:49:42 +0800
From:      Kevin Lo <kevlo@FreeBSD.org>
To:        Gleb Smirnoff <glebius@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r260501 - head/sys/dev/usb/wlan
Message-ID:  <52D00886.9090603@FreeBSD.org>
In-Reply-To: <20140110102032.GC73147@FreeBSD.org>
References:  <201401100247.s0A2lK9q066888@svn.freebsd.org> <20140110102032.GC73147@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On 2014/01/10 18:20, Gleb Smirnoff wrote:
>   Kevin,
>
> On Fri, Jan 10, 2014 at 02:47:20AM +0000, Kevin Lo wrote:
> K> Author: kevlo
> K> Date: Fri Jan 10 02:47:20 2014
> K> New Revision: 260501
> K> URL: http://svnweb.freebsd.org/changeset/base/260501
> K>
> K> Log:
> K>   Use m_getcl() instead of MGETHDR/MCLGET macros.
> K>
> K>   Suggested by:	glebius
> K>
> K> Modified:
> K>   head/sys/dev/usb/wlan/if_rsu.c
> K>
> K> Modified: head/sys/dev/usb/wlan/if_rsu.c
> K> ==============================================================================
> K> --- head/sys/dev/usb/wlan/if_rsu.c	Fri Jan 10 01:44:34 2014	(r260500)
> K> +++ head/sys/dev/usb/wlan/if_rsu.c	Fri Jan 10 02:47:20 2014	(r260501)
> K> @@ -1145,16 +1145,9 @@ rsu_event_survey(struct rsu_softc *sc, u
> K>  	pktlen = sizeof(*wh) + le32toh(bss->ieslen);
> K>  	if (__predict_false(pktlen > MCLBYTES))
> K>  		return;
> K> -	MGETHDR(m, M_NOWAIT, MT_DATA);
> K> +	m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR);
> K>  	if (__predict_false(m == NULL))
> K>  		return;
> K> -	if (pktlen > MHLEN) {
> K> -		MCLGET(m, M_NOWAIT);
> K> -		if (!(m->m_flags & M_EXT)) {
> K> -			m_free(m);
> K> -			return;
> K> -		}
> K> -	}
> K>  	wh = mtod(m, struct ieee80211_frame *);
> K>  	wh->i_fc[0] = IEEE80211_FC0_VERSION_0 | IEEE80211_FC0_TYPE_MGT |
> K>  	    IEEE80211_FC0_SUBTYPE_BEACON;
> K> @@ -1358,19 +1351,11 @@ rsu_rx_frame(struct rsu_softc *sc, uint8
> K>  	DPRINTFN(5, "Rx frame len=%d rate=%d infosz=%d rssi=%d\n",
> K>  	    pktlen, rate, infosz, *rssi);
> K>
> K> -	MGETHDR(m, M_NOWAIT, MT_DATA);
> K> +	m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR);
> K>  	if (__predict_false(m == NULL)) {
> K>  		ifp->if_ierrors++;
> K>  		return NULL;
> K>  	}
> K> -	if (pktlen > MHLEN) {
> K> -		MCLGET(m, M_NOWAIT);
> K> -		if (__predict_false(!(m->m_flags & M_EXT))) {
> K> -			ifp->if_ierrors++;
> K> -			m_freem(m);
> K> -			return NULL;
> K> -		}
> K> -	}
> K>  	/* Finalize mbuf. */
> K>  	m->m_pkthdr.rcvif = ifp;
> K>  	/* Hardware does Rx TCP checksum offload. */
>
> Sorry, but the correct code would be:
>
> 	if (pktlen > MHLEN)
> 		m = m_getcl(M_NOWAIT, MT_DATA, M_PKTHDR);
> 	else
> 		m = m_gethdr(M_NOWAIT, MT_DATA);
> 	if (__predict_false(m == NULL)) {
> 		ifp->if_ierrors++;
> 		return NULL;
> 	}
>
> Alternatively, you can use:
>
> 	m = m_get2(pktlen, M_NOWAIT, MT_DATA, M_PKTHDR);
> 	if (__predict_false(m == NULL)) {
> 		ifp->if_ierrors++;
> 		return NULL;
> 	}
>
> With committed code we are wasting memory for small packets.
>

Fixed.  Thanks for pointing that out!
Seems like I was not clear headed. I thought I used m_get2 but 
apparently not.

     Kevin




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52D00886.9090603>