svn commit: r274459 - head/sys/dev/netmap
Adrian Chadd
adrian at freebsd.org
Thu Nov 13 00:48:26 UTC 2014
Thanks!
This does some pretty impressive lock contention reducing. It's not
zero (it's the same kind of producer/consumer contention that's
currently problematic) but it's much less than before. This means that
netmap can be used with multiple rings on multiple CPUs.
I'll tackle profiling poll() next with luigi and kib's suggested changes.
Thanks again!
-adrian
On 12 November 2014 16:40, Luigi Rizzo <luigi at freebsd.org> wrote:
> Author: luigi
> Date: Thu Nov 13 00:40:34 2014
> New Revision: 274459
> URL: https://svnweb.freebsd.org/changeset/base/274459
>
> Log:
> add support for private knote lock (reduces lock contention),
> adapting OS_selrecord accordingly.
> Problem and fix suggested by adrian and jmg
>
> Modified:
> head/sys/dev/netmap/netmap.c
> head/sys/dev/netmap/netmap_freebsd.c
> head/sys/dev/netmap/netmap_kern.h
>
> Modified: head/sys/dev/netmap/netmap.c
> ==============================================================================
> --- head/sys/dev/netmap/netmap.c Thu Nov 13 00:30:17 2014 (r274458)
> +++ head/sys/dev/netmap/netmap.c Thu Nov 13 00:40:34 2014 (r274459)
> @@ -375,9 +375,14 @@ ports attached to the switch)
>
> /* reduce conditional code */
> // linux API, use for the knlist in FreeBSD
> -#define init_waitqueue_head(x) knlist_init_mtx(&(x)->si_note, NULL)
> +/* use a private mutex for the knlist */
> +#define init_waitqueue_head(x) do { \
> + struct mtx *m = &(x)->m; \
> + mtx_init(m, "nm_kn_lock", NULL, MTX_DEF); \
> + knlist_init_mtx(&(x)->si.si_note, m); \
> + } while (0)
>
> -void freebsd_selwakeup(struct selinfo *si, int pri);
> +#define OS_selrecord(a, b) selrecord(a, &((b)->si))
> #define OS_selwakeup(a, b) freebsd_selwakeup(a, b)
>
> #elif defined(linux)
> @@ -806,6 +811,19 @@ netmap_krings_create(struct netmap_adapt
> }
>
>
> +#ifdef __FreeBSD__
> +static void
> +netmap_knlist_destroy(NM_SELINFO_T *si)
> +{
> + /* XXX kqueue(9) needed; these will mirror knlist_init. */
> + knlist_delete(&si->si.si_note, curthread, 0 /* not locked */ );
> + knlist_destroy(&si->si.si_note);
> + /* now we don't need the mutex anymore */
> + mtx_destroy(&si->m);
> +}
> +#endif /* __FreeBSD__ */
> +
> +
> /* undo the actions performed by netmap_krings_create */
> /* call with NMG_LOCK held */
> void
> @@ -816,6 +834,7 @@ netmap_krings_delete(struct netmap_adapt
> /* we rely on the krings layout described above */
> for ( ; kring != na->tailroom; kring++) {
> mtx_destroy(&kring->q_lock);
> + netmap_knlist_destroy(&kring->si);
> }
> free(na->tx_rings, M_DEVBUF);
> na->tx_rings = na->rx_rings = na->tailroom = NULL;
> @@ -996,9 +1015,8 @@ netmap_do_unregif(struct netmap_priv_d *
> * XXX The wake up now must happen during *_down(), when
> * we order all activities to stop. -gl
> */
> - /* XXX kqueue(9) needed; these will mirror knlist_init. */
> - /* knlist_destroy(&na->tx_si.si_note); */
> - /* knlist_destroy(&na->rx_si.si_note); */
> + netmap_knlist_destroy(&na->tx_si);
> + netmap_knlist_destroy(&na->rx_si);
>
> /* delete rings and buffers */
> netmap_mem_rings_delete(na);
> @@ -1310,7 +1328,7 @@ netmap_rxsync_from_host(struct netmap_ad
>
> /* access copies of cur,tail in the kring */
> if (kring->rcur == kring->rtail && td) /* no bufs available */
> - selrecord(td, &kring->si);
> + OS_selrecord(td, &kring->si);
>
> mbq_unlock(q);
> return ret;
> @@ -2410,7 +2428,7 @@ flush_tx:
> }
> }
> if (want_tx && retry_tx && !is_kevent) {
> - selrecord(td, check_all_tx ?
> + OS_selrecord(td, check_all_tx ?
> &na->tx_si : &na->tx_rings[priv->np_txqfirst].si);
> retry_tx = 0;
> goto flush_tx;
> @@ -2479,7 +2497,7 @@ do_retry_rx:
> }
>
> if (retry_rx && !is_kevent)
> - selrecord(td, check_all_rx ?
> + OS_selrecord(td, check_all_rx ?
> &na->rx_si : &na->rx_rings[priv->np_rxqfirst].si);
> if (send_down > 0 || retry_rx) {
> retry_rx = 0;
> @@ -3054,8 +3072,15 @@ netmap_init(void)
> if (error != 0)
> goto fail;
> /* XXX could use make_dev_credv() to get error number */
> +#ifdef __FreeBSD__
> + /* support for the 'eternal' flag */
> + netmap_dev = make_dev_credf(MAKEDEV_ETERNAL_KLD,
> + &netmap_cdevsw, 0, NULL, UID_ROOT, GID_WHEEL, 0660,
> + "netmap");
> +#else
> netmap_dev = make_dev(&netmap_cdevsw, 0, UID_ROOT, GID_WHEEL, 0660,
> "netmap");
> +#endif
> if (!netmap_dev)
> goto fail;
>
>
> Modified: head/sys/dev/netmap/netmap_freebsd.c
> ==============================================================================
> --- head/sys/dev/netmap/netmap_freebsd.c Thu Nov 13 00:30:17 2014 (r274458)
> +++ head/sys/dev/netmap/netmap_freebsd.c Thu Nov 13 00:40:34 2014 (r274459)
> @@ -656,25 +656,24 @@ netmap_open(struct cdev *dev, int oflags
> * and do not need the selrecord().
> */
>
> -void freebsd_selwakeup(struct selinfo *si, int pri);
>
> void
> -freebsd_selwakeup(struct selinfo *si, int pri)
> +freebsd_selwakeup(struct nm_selinfo *si, int pri)
> {
> if (netmap_verbose)
> - D("on knote %p", &si->si_note);
> - selwakeuppri(si, pri);
> + D("on knote %p", &si->si.si_note);
> + selwakeuppri(&si->si, pri);
> /* use a non-zero hint to tell the notification from the
> * call done in kqueue_scan() which uses 0
> */
> - KNOTE_UNLOCKED(&si->si_note, 0x100 /* notification */);
> + KNOTE_UNLOCKED(&si->si.si_note, 0x100 /* notification */);
> }
>
> static void
> netmap_knrdetach(struct knote *kn)
> {
> struct netmap_priv_d *priv = (struct netmap_priv_d *)kn->kn_hook;
> - struct selinfo *si = priv->np_rxsi;
> + struct selinfo *si = &priv->np_rxsi->si;
>
> D("remove selinfo %p", si);
> knlist_remove(&si->si_note, kn, 0);
> @@ -684,7 +683,7 @@ static void
> netmap_knwdetach(struct knote *kn)
> {
> struct netmap_priv_d *priv = (struct netmap_priv_d *)kn->kn_hook;
> - struct selinfo *si = priv->np_txsi;
> + struct selinfo *si = &priv->np_txsi->si;
>
> D("remove selinfo %p", si);
> knlist_remove(&si->si_note, kn, 0);
> @@ -756,7 +755,7 @@ netmap_kqfilter(struct cdev *dev, struct
> struct netmap_priv_d *priv;
> int error;
> struct netmap_adapter *na;
> - struct selinfo *si;
> + struct nm_selinfo *si;
> int ev = kn->kn_filter;
>
> if (ev != EVFILT_READ && ev != EVFILT_WRITE) {
> @@ -779,7 +778,7 @@ netmap_kqfilter(struct cdev *dev, struct
> kn->kn_fop = (ev == EVFILT_WRITE) ?
> &netmap_wfiltops : &netmap_rfiltops;
> kn->kn_hook = priv;
> - knlist_add(&si->si_note, kn, 1);
> + knlist_add(&si->si.si_note, kn, 1);
> // XXX unlock(priv)
> ND("register %p %s td %p priv %p kn %p np_nifp %p kn_fp/fpop %s",
> na, na->ifp->if_xname, curthread, priv, kn,
>
> Modified: head/sys/dev/netmap/netmap_kern.h
> ==============================================================================
> --- head/sys/dev/netmap/netmap_kern.h Thu Nov 13 00:30:17 2014 (r274458)
> +++ head/sys/dev/netmap/netmap_kern.h Thu Nov 13 00:40:34 2014 (r274459)
> @@ -55,7 +55,7 @@
> #define NMG_UNLOCK() sx_xunlock(&netmap_global_lock)
> #define NMG_LOCK_ASSERT() sx_assert(&netmap_global_lock, SA_XLOCKED)
>
> -#define NM_SELINFO_T struct selinfo
> +#define NM_SELINFO_T struct nm_selinfo
> #define MBUF_LEN(m) ((m)->m_pkthdr.len)
> #define MBUF_IFP(m) ((m)->m_pkthdr.rcvif)
> #define NM_SEND_UP(ifp, m) ((NA(ifp))->if_input)(ifp, m)
> @@ -88,6 +88,13 @@ struct netmap_adapter *netmap_getna(if_t
>
> MALLOC_DECLARE(M_NETMAP);
>
> +struct nm_selinfo {
> + struct selinfo si;
> + struct mtx m;
> +};
> +
> +void freebsd_selwakeup(struct nm_selinfo *si, int pri);
> +
> // XXX linux struct, not used in FreeBSD
> struct net_device_ops {
> };
>
More information about the svn-src-all
mailing list