Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 Jan 2014 09:31:00 -0800
From:      Adrian Chadd <adrian@freebsd.org>
To:        "Andrey V. Elsukov" <ae@freebsd.org>
Cc:        svn-src-stable@freebsd.org, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, svn-src-stable-10@freebsd.org
Subject:   Re: svn commit: r260504 - in stable/10/sys: netinet netinet6
Message-ID:  <CAJ-VmonYWF6ebVcd07CFwT2Lt%2BE2i7Yeo00w6RhYeM3bvDbSOw@mail.gmail.com>
In-Reply-To: <201401100945.s0A9jSUk028492@svn.freebsd.org>
References:  <201401100945.s0A9jSUk028492@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hah, thanks for beating me to the MFC. :)


-a

On 10 January 2014 01:45, Andrey V. Elsukov <ae@freebsd.org> wrote:
> Author: ae
> Date: Fri Jan 10 09:45:28 2014
> New Revision: 260504
> URL: http://svnweb.freebsd.org/changeset/base/260504
>
> Log:
>   MFC r260151 (by adrian):
>     Use an RLOCK here instead of an RWLOCK - matching all the other calls
>     to lla_lookup().
>
>     This drastically reduces the very high lock contention when doing parallel
>     TCP throughput tests (> 1024 sockets) with IPv6.
>
>   MFC r260187:
>     lla_lookup() does modification only when LLE_CREATE is specified.
>     Thus we can use IF_AFDATA_RLOCK() instead of IF_AFDATA_LOCK() when doing
>     lla_lookup() without LLE_CREATE flag.
>
>   MFC r260217:
>     Add IF_AFDATA_WLOCK_ASSERT() in case lla_lookup() is called with
>     LLE_CREATE flag.
>
> Modified:
>   stable/10/sys/netinet/if_ether.c
>   stable/10/sys/netinet/in.c
>   stable/10/sys/netinet6/in6.c
>   stable/10/sys/netinet6/nd6.c
>   stable/10/sys/netinet6/nd6_nbr.c
> Directory Properties:
>   stable/10/   (props changed)
>
> Modified: stable/10/sys/netinet/if_ether.c
> ==============================================================================
> --- stable/10/sys/netinet/if_ether.c    Fri Jan 10 07:48:36 2014        (r260503)
> +++ stable/10/sys/netinet/if_ether.c    Fri Jan 10 09:45:28 2014        (r260504)
> @@ -153,10 +153,10 @@ arp_ifscrub(struct ifnet *ifp, uint32_t
>         addr4.sin_len    = sizeof(addr4);
>         addr4.sin_family = AF_INET;
>         addr4.sin_addr.s_addr = addr;
> -       IF_AFDATA_LOCK(ifp);
> +       IF_AFDATA_RLOCK(ifp);
>         lla_lookup(LLTABLE(ifp), (LLE_DELETE | LLE_IFADDR),
>             (struct sockaddr *)&addr4);
> -       IF_AFDATA_UNLOCK(ifp);
> +       IF_AFDATA_RUNLOCK(ifp);
>  }
>  #endif
>
> @@ -805,9 +805,9 @@ reply:
>                 struct llentry *lle = NULL;
>
>                 sin.sin_addr = itaddr;
> -               IF_AFDATA_LOCK(ifp);
> +               IF_AFDATA_RLOCK(ifp);
>                 lle = lla_lookup(LLTABLE(ifp), 0, (struct sockaddr *)&sin);
> -               IF_AFDATA_UNLOCK(ifp);
> +               IF_AFDATA_RUNLOCK(ifp);
>
>                 if ((lle != NULL) && (lle->la_flags & LLE_PUB)) {
>                         (void)memcpy(ar_tha(ah), ar_sha(ah), ah->ar_hln);
>
> Modified: stable/10/sys/netinet/in.c
> ==============================================================================
> --- stable/10/sys/netinet/in.c  Fri Jan 10 07:48:36 2014        (r260503)
> +++ stable/10/sys/netinet/in.c  Fri Jan 10 09:45:28 2014        (r260504)
> @@ -1435,6 +1435,7 @@ in_lltable_lookup(struct lltable *llt, u
>  #endif
>                 if (!(flags & LLE_CREATE))
>                         return (NULL);
> +               IF_AFDATA_WLOCK_ASSERT(ifp);
>                 /*
>                  * A route that covers the given address must have
>                  * been installed 1st because we are doing a resolution,
>
> Modified: stable/10/sys/netinet6/in6.c
> ==============================================================================
> --- stable/10/sys/netinet6/in6.c        Fri Jan 10 07:48:36 2014        (r260503)
> +++ stable/10/sys/netinet6/in6.c        Fri Jan 10 09:45:28 2014        (r260504)
> @@ -2627,6 +2627,7 @@ in6_lltable_lookup(struct lltable *llt,
>         if (lle == NULL) {
>                 if (!(flags & LLE_CREATE))
>                         return (NULL);
> +               IF_AFDATA_WLOCK_ASSERT(ifp);
>                 /*
>                  * A route that covers the given address must have
>                  * been installed 1st because we are doing a resolution,
>
> Modified: stable/10/sys/netinet6/nd6.c
> ==============================================================================
> --- stable/10/sys/netinet6/nd6.c        Fri Jan 10 07:48:36 2014        (r260503)
> +++ stable/10/sys/netinet6/nd6.c        Fri Jan 10 09:45:28 2014        (r260504)
> @@ -1146,9 +1146,9 @@ nd6_nud_hint(struct rtentry *rt, struct
>                 return;
>
>         ifp = rt->rt_ifp;
> -       IF_AFDATA_LOCK(ifp);
> +       IF_AFDATA_RLOCK(ifp);
>         ln = nd6_lookup(dst6, ND6_EXCLUSIVE, NULL);
> -       IF_AFDATA_UNLOCK(ifp);
> +       IF_AFDATA_RUNLOCK(ifp);
>         if (ln == NULL)
>                 return;
>
> @@ -1574,16 +1574,16 @@ nd6_cache_lladdr(struct ifnet *ifp, stru
>          * description on it in NS section (RFC 2461 7.2.3).
>          */
>         flags = lladdr ? ND6_EXCLUSIVE : 0;
> -       IF_AFDATA_LOCK(ifp);
> +       IF_AFDATA_RLOCK(ifp);
>         ln = nd6_lookup(from, flags, ifp);
> -
> +       IF_AFDATA_RUNLOCK(ifp);
>         if (ln == NULL) {
>                 flags |= ND6_EXCLUSIVE;
> +               IF_AFDATA_LOCK(ifp);
>                 ln = nd6_lookup(from, flags | ND6_CREATE, ifp);
>                 IF_AFDATA_UNLOCK(ifp);
>                 is_newentry = 1;
>         } else {
> -               IF_AFDATA_UNLOCK(ifp);
>                 /* do nothing if static ndp is set */
>                 if (ln->la_flags & LLE_STATIC) {
>                         static_route = 1;
> @@ -1891,9 +1891,9 @@ nd6_output_lle(struct ifnet *ifp, struct
>         flags = ((m != NULL) || (lle != NULL)) ? LLE_EXCLUSIVE : 0;
>         if (ln == NULL) {
>         retry:
> -               IF_AFDATA_LOCK(ifp);
> +               IF_AFDATA_RLOCK(ifp);
>                 ln = lla_lookup(LLTABLE6(ifp), flags, (struct sockaddr *)dst);
> -               IF_AFDATA_UNLOCK(ifp);
> +               IF_AFDATA_RUNLOCK(ifp);
>                 if ((ln == NULL) && nd6_is_addr_neighbor(dst, ifp))  {
>                         /*
>                          * Since nd6_is_addr_neighbor() internally calls nd6_lookup(),
>
> Modified: stable/10/sys/netinet6/nd6_nbr.c
> ==============================================================================
> --- stable/10/sys/netinet6/nd6_nbr.c    Fri Jan 10 07:48:36 2014        (r260503)
> +++ stable/10/sys/netinet6/nd6_nbr.c    Fri Jan 10 09:45:28 2014        (r260504)
> @@ -736,9 +736,9 @@ nd6_na_input(struct mbuf *m, int off, in
>          * If no neighbor cache entry is found, NA SHOULD silently be
>          * discarded.
>          */
> -       IF_AFDATA_LOCK(ifp);
> +       IF_AFDATA_RLOCK(ifp);
>         ln = nd6_lookup(&taddr6, LLE_EXCLUSIVE, ifp);
> -       IF_AFDATA_UNLOCK(ifp);
> +       IF_AFDATA_RUNLOCK(ifp);
>         if (ln == NULL) {
>                 goto freeit;
>         }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmonYWF6ebVcd07CFwT2Lt%2BE2i7Yeo00w6RhYeM3bvDbSOw>